On Tue, Nov 27, 2018 at 04:13:30PM +0530, Ramalingam C wrote: > Commits the content protection change of a connector, > without crtc modeset. This improves the user experience. > > Originally proposed by Sean Paul at v3 of HDCP1.4 framework > https://patchwork.freedesktop.org/patch/191759/. For some > reason this was dropped, but needed for the proper functionality > of HDCP encryption. > > Signed-off-by: Ramalingam C <ramalingam.c@xxxxxxxxx> > Suggested-by: Sean Paul <seanpaul@xxxxxxxxxxxx> I think best to split this out from this series, since it's a separate concern really. Wrt implementation, this problem here is pretty similar to other sink setup stuff like infoframes, where atm we also require a full modeset. The idea that iirc Ville&me toyed around with is a kind of per-encoder/connector fixup function, where all this stuff could be set if we did _not_ do a full modeset. For a full modeset I think it's best to still keep all this in connectors. Plus for anything where we do this kind of fast-modeset trick we need to lock down the state tracking using the state readout code. Otherwise this is very hard to validate properly. -Daniel > --- > drivers/gpu/drm/i915/intel_ddi.c | 7 ------- > drivers/gpu/drm/i915/intel_display.c | 10 ++++++++++ > drivers/gpu/drm/i915/intel_drv.h | 5 +++++ > drivers/gpu/drm/i915/intel_hdcp.c | 32 ++++++++++++++++++++++++++++---- > 4 files changed, 43 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index ad11540ac436..128cc558d75f 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -3469,11 +3469,6 @@ static void intel_enable_ddi(struct intel_encoder *encoder, > intel_enable_ddi_hdmi(encoder, crtc_state, conn_state); > else > intel_enable_ddi_dp(encoder, crtc_state, conn_state); > - > - /* Enable hdcp if it's desired */ > - if (conn_state->content_protection == > - DRM_MODE_CONTENT_PROTECTION_DESIRED) > - intel_hdcp_enable(to_intel_connector(conn_state->connector)); > } > > static void intel_disable_ddi_dp(struct intel_encoder *encoder, > @@ -3513,8 +3508,6 @@ static void intel_disable_ddi(struct intel_encoder *encoder, > const struct intel_crtc_state *old_crtc_state, > const struct drm_connector_state *old_conn_state) > { > - intel_hdcp_disable(to_intel_connector(old_conn_state->connector)); > - > if (intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_HDMI)) > intel_disable_ddi_hdmi(encoder, old_crtc_state, old_conn_state); > else > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index fc63babce165..98aaf536146f 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -12854,6 +12854,8 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state) > struct drm_i915_private *dev_priv = to_i915(dev); > struct drm_crtc_state *old_crtc_state, *new_crtc_state; > struct intel_crtc_state *new_intel_crtc_state, *old_intel_crtc_state; > + struct drm_connector_state *old_conn_state, *new_conn_state; > + struct drm_connector *connector; > struct drm_crtc *crtc; > struct intel_crtc *intel_crtc; > u64 put_domains[I915_MAX_PIPES] = {}; > @@ -12947,9 +12949,17 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state) > } > } > > + for_each_oldnew_connector_in_state(state, connector, old_conn_state, > + new_conn_state, i) > + intel_hdcp_atomic_pre_commit(connector, old_conn_state, > + new_conn_state); > + > /* Now enable the clocks, plane, pipe, and connectors that we set up. */ > dev_priv->display.update_crtcs(state); > > + for_each_new_connector_in_state(state, connector, new_conn_state, i) > + intel_hdcp_atomic_commit(connector, new_conn_state); > + > /* FIXME: We should call drm_atomic_helper_commit_hw_done() here > * already, but still need the state for the delayed optimization. To > * fix this: > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 2b7181b76475..68eae1e569e4 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -2084,6 +2084,11 @@ static inline void intel_backlight_device_unregister(struct intel_connector *con > void intel_hdcp_atomic_check(struct drm_connector *connector, > struct drm_connector_state *old_state, > struct drm_connector_state *new_state); > +void intel_hdcp_atomic_pre_commit(struct drm_connector *connector, > + struct drm_connector_state *old_state, > + struct drm_connector_state *new_state); > +void intel_hdcp_atomic_commit(struct drm_connector *connector, > + struct drm_connector_state *new_state); > int intel_hdcp_init(struct intel_connector *connector, > const struct intel_hdcp_shim *hdcp_shim, > bool hdcp2_supported); > diff --git a/drivers/gpu/drm/i915/intel_hdcp.c b/drivers/gpu/drm/i915/intel_hdcp.c > index be475914c3cd..9f4056e156ec 100644 > --- a/drivers/gpu/drm/i915/intel_hdcp.c > +++ b/drivers/gpu/drm/i915/intel_hdcp.c > @@ -2005,7 +2005,6 @@ void intel_hdcp_atomic_check(struct drm_connector *connector, > { > uint64_t old_cp = old_state->content_protection; > uint64_t new_cp = new_state->content_protection; > - struct drm_crtc_state *crtc_state; > > if (!new_state->crtc) { > /* > @@ -2026,10 +2025,35 @@ void intel_hdcp_atomic_check(struct drm_connector *connector, > (old_cp == DRM_MODE_CONTENT_PROTECTION_DESIRED && > new_cp == DRM_MODE_CONTENT_PROTECTION_ENABLED)) > return; > +} > + > +void intel_hdcp_atomic_pre_commit(struct drm_connector *connector, > + struct drm_connector_state *old_state, > + struct drm_connector_state *new_state) > +{ > + u64 old_cp = old_state->content_protection; > + u64 new_cp = new_state->content_protection; > + > + /* > + * Disable HDCP if the connector is becoming disabled, or if requested > + * via the property. > + */ > + if ((!new_state->crtc && > + old_cp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED) || > + (new_state->crtc && > + old_cp != DRM_MODE_CONTENT_PROTECTION_UNDESIRED && > + new_cp == DRM_MODE_CONTENT_PROTECTION_UNDESIRED)) > + intel_hdcp_disable(to_intel_connector(connector)); > +} > + > +void intel_hdcp_atomic_commit(struct drm_connector *connector, > + struct drm_connector_state *new_state) > +{ > + u64 new_cp = new_state->content_protection; > > - crtc_state = drm_atomic_get_new_crtc_state(new_state->state, > - new_state->crtc); > - crtc_state->mode_changed = true; > + /* Enable hdcp if it's desired */ > + if (new_state->crtc && new_cp == DRM_MODE_CONTENT_PROTECTION_DESIRED) > + intel_hdcp_enable(to_intel_connector(connector)); > } > > /* Handles the CP_IRQ raised from the DP HDCP sink */ > -- > 2.7.4 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel