| Message ID | 1548917996-28081-38-git-send-email-ramalingam.c@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Implement HDCP2.2 | expand |
On Thu, Jan 31, 2019 at 12:29:53PM +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@intel.com> > Suggested-by: Sean Paul <seanpaul@chromium.org> > --- > 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 ca705546a0ab..9cb03c1b0abc 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -3495,11 +3495,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, > @@ -3542,8 +3537,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 a025efb1d7c6..9b964dabb57c 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -13034,6 +13034,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] = {}; > @@ -13128,9 +13130,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); So this is kinda awkward, having these one-use callbacks for hdcp. "changing connector state without a full modeset" is a generic problem, and Hans has added a new intel_encoder->update_pipe callback for this stuff. The way to use it: - Keep the enable/disable calls where they currently are, we still need them for full modesets. - Add a new update_pipe callback, which is for the !needs_modeset case for both hdmi and dp, and adjust hdcp state there. - Adjust compute_config to only flag update_pipe, not a full modeset in compute_config (in atomic_check). This should fit a lot neater into the overall infrastructure for fastset and all that stuff. For validation the problem is that we're now adding two paths, so need to make sure the content protection test copes. We need to make sure that we're enabling content protection both with a modeset and without, probably the simplest trick to get there is to add a new subtest which does a dpms off->on once content protection is on, and checks that it stays on. Also we need CI results without this patch so I can start merging. Rough merge plan: - needs ack to merge component.c through drm-intel - merge all the i915 patches - topic branch for mei, shared with mei subsystem - make sure that CI has mei enabled and that tests aren't on fire - merge this one here Cheers, Daniel > + > /* 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 1901d12bacc4..e6b6e920ab01 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -2129,6 +2129,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 f38feeadb363..ef127dc7a241 100644 > --- a/drivers/gpu/drm/i915/intel_hdcp.c > +++ b/drivers/gpu/drm/i915/intel_hdcp.c > @@ -1876,7 +1876,6 @@ void intel_hdcp_atomic_check(struct drm_connector *connector, > { > u64 old_cp = old_state->content_protection; > u64 new_cp = new_state->content_protection; > - struct drm_crtc_state *crtc_state; > > if (!new_state->crtc) { > /* > @@ -1897,10 +1896,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 >
Tomas, On 1/31/2019 2:02 PM, Daniel Vetter wrote: > Also we need CI results without this patch so I can start merging. Rough > merge plan: > - needs ack to merge component.c through drm-intel > - merge all the i915 patches > - topic branch for mei, shared with mei subsystem Is this convenient to you? Or you have any other suggestion? --Ram > - make sure that CI has mei enabled and that tests aren't on fire > - merge this one here > > Cheers, Daniel
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index ca705546a0ab..9cb03c1b0abc 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -3495,11 +3495,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, @@ -3542,8 +3537,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 a025efb1d7c6..9b964dabb57c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13034,6 +13034,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] = {}; @@ -13128,9 +13130,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 1901d12bacc4..e6b6e920ab01 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -2129,6 +2129,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 f38feeadb363..ef127dc7a241 100644 --- a/drivers/gpu/drm/i915/intel_hdcp.c +++ b/drivers/gpu/drm/i915/intel_hdcp.c @@ -1876,7 +1876,6 @@ void intel_hdcp_atomic_check(struct drm_connector *connector, { u64 old_cp = old_state->content_protection; u64 new_cp = new_state->content_protection; - struct drm_crtc_state *crtc_state; if (!new_state->crtc) { /* @@ -1897,10 +1896,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 */
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@intel.com> Suggested-by: Sean Paul <seanpaul@chromium.org> --- 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(-)