On Thu, 05 Mar 2020, Rajat Jain <rajatja@xxxxxxxxxx> wrote: > OK, will do. In order to do that I may need to introduce driver level > hooks that i915 driver can populate and drm core can call (or may be > some functions to add privacy screen property that drm core exports > and i915 driver will call). The latter. Look at drm_connector_attach_*() functions in drm_connector.c. i915 (or any other driver) can create and attach the property as needed. drm_atomic_connector_{get,set}_property in drm_atomic_uapi.c need to handle the properties, but *only* to get/set the value in drm_connector_state, nothing more. How that value is actually used is up to the drivers, but the userspace interface will be the same instead of being driver specific. >> > @@ -93,15 +97,18 @@ int intel_digital_connector_atomic_set_property(struct drm_connector *connector, >> > struct drm_i915_private *dev_priv = to_i915(dev); >> > struct intel_digital_connector_state *intel_conn_state = >> > to_intel_digital_connector_state(state); >> > + struct intel_connector *intel_connector = to_intel_connector(connector); >> > >> > if (property == dev_priv->force_audio_property) { >> > intel_conn_state->force_audio = val; >> > return 0; >> > - } >> > - >> > - if (property == dev_priv->broadcast_rgb_property) { >> > + } else if (property == dev_priv->broadcast_rgb_property) { >> > intel_conn_state->broadcast_rgb = val; >> > return 0; >> > + } else if (property == intel_connector->privacy_screen_property) { >> > + intel_privacy_screen_set_val(intel_connector, val); >> >> I think this part should only change the connector state. The driver >> would then do the magic at commit stage according to the property value. Also, this would be the part that's done in drm core level. > Can you please point me to some code reference as to where in code > does the "commit stage" apply the changes? Look at, say, drm_connector_attach_scaling_mode_property(). In the getter/setter code it'll just read/change state->scaling_mode. You can use the value in encoder ->enable, ->disable, and ->update_pipe hooks. Enable should enable the privacy screen if desired, disable should probably unconditionally disable the privacy screen while disabling the display, and update should just change the state according to the value. Update is called if there isn't a full modeset. (Scaling mode is a bit more indirect than that, affecting other things in the encoder ->compute_config hook, leading to similar effects.) Ville, anything I missed? BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel