| Message ID | 20201023122112.15265-2-anshuman.gupta@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,01/16] drm/i915/hdcp: Update CP property in update_pipe | expand |
> -----Original Message----- > From: Anshuman Gupta <anshuman.gupta@intel.com> > Sent: Friday, October 23, 2020 5:51 PM > To: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org > Cc: seanpaul@chromium.org; Nikula, Jani <jani.nikula@intel.com>; C, > Ramalingam <ramalingam.c@intel.com>; Li, Juston <juston.li@intel.com>; > Shankar, Uma <uma.shankar@intel.com>; Gupta, Anshuman > <anshuman.gupta@intel.com> > Subject: [PATCH v3 01/16] drm/i915/hdcp: Update CP property in update_pipe > > When crtc state need_modeset is true it is not necessary it is going to be a real > modeset, it can turns to be a update_pipe instead of modeset. I believe you refer fastest here. May be make this a bit clear. > This turns content protection property to be DESIRED and hdcp update_pipe left > with property to be in DESIRED state but actually hdcp->value was ENABLED. > This caught with DP MST setup, when disabling HDCP on a connector sets the crtc > state need_modeset to true for all crtc driving the other DP-MST topology > connectors. This is a bit ambiguous, you can mention it a bit more clearly. In case of DP MST, how this affects would help make it clearer. > > v2: > Fix WARN_ON(connector->base.registration_state == > DRM_CONNECTOR_REGISTERED) > > Fixes: 33f9a623bfc6 ("drm/i915/hdcp: Update CP as per the kernel internal > state") > Cc: Ramalingam C <ramalingam.c@intel.com> > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com> > --- > drivers/gpu/drm/i915/display/intel_hdcp.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c > b/drivers/gpu/drm/i915/display/intel_hdcp.c > index b2a4bbcfdcd2..0d9e8d3b5603 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > @@ -2221,6 +2221,11 @@ void intel_hdcp_update_pipe(struct > intel_atomic_state *state, > desired_and_not_enabled = > hdcp->value != > DRM_MODE_CONTENT_PROTECTION_ENABLED; > mutex_unlock(&hdcp->mutex); > Please add a comment explaining the rationale here as well. > + if (!desired_and_not_enabled && > !content_protection_type_changed) { > + drm_connector_get(&connector->base); Where are we releasing this ref. > + schedule_work(&hdcp->prop_work); > + } > } > > if (desired_and_not_enabled || content_protection_type_changed) > -- > 2.26.2
On 2020-10-27 at 11:02:26 +0530, Shankar, Uma wrote: > > > > -----Original Message----- > > From: Anshuman Gupta <anshuman.gupta@intel.com> > > Sent: Friday, October 23, 2020 5:51 PM > > To: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org > > Cc: seanpaul@chromium.org; Nikula, Jani <jani.nikula@intel.com>; C, > > Ramalingam <ramalingam.c@intel.com>; Li, Juston <juston.li@intel.com>; > > Shankar, Uma <uma.shankar@intel.com>; Gupta, Anshuman > > <anshuman.gupta@intel.com> > > Subject: [PATCH v3 01/16] drm/i915/hdcp: Update CP property in update_pipe > > > > When crtc state need_modeset is true it is not necessary it is going to be a real > > modeset, it can turns to be a update_pipe instead of modeset. > > I believe you refer fastest here. May be make this a bit clear. > > > This turns content protection property to be DESIRED and hdcp update_pipe left > > with property to be in DESIRED state but actually hdcp->value was ENABLED. > > This caught with DP MST setup, when disabling HDCP on a connector sets the crtc > > state need_modeset to true for all crtc driving the other DP-MST topology > > connectors. > > This is a bit ambiguous, you can mention it a bit more clearly. In case of DP MST, how this > affects would help make it clearer. > > > > > v2: > > Fix WARN_ON(connector->base.registration_state == > > DRM_CONNECTOR_REGISTERED) > > > > Fixes: 33f9a623bfc6 ("drm/i915/hdcp: Update CP as per the kernel internal > > state") > > Cc: Ramalingam C <ramalingam.c@intel.com> > > Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_hdcp.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c > > b/drivers/gpu/drm/i915/display/intel_hdcp.c > > index b2a4bbcfdcd2..0d9e8d3b5603 100644 > > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > > @@ -2221,6 +2221,11 @@ void intel_hdcp_update_pipe(struct > > intel_atomic_state *state, > > desired_and_not_enabled = > > hdcp->value != > > DRM_MODE_CONTENT_PROTECTION_ENABLED; > > mutex_unlock(&hdcp->mutex); > > > > Please add a comment explaining the rationale here as well. Sure i will fix all above comment. > > > + if (!desired_and_not_enabled && > > !content_protection_type_changed) { > > + drm_connector_get(&connector->base); > > Where are we releasing this ref. prop worker function releases the connector reference. Thanks, Anshuman Gupta. > > > + schedule_work(&hdcp->prop_work); > > + } > > } > > > > if (desired_and_not_enabled || content_protection_type_changed) > > -- > > 2.26.2 >
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c index b2a4bbcfdcd2..0d9e8d3b5603 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c @@ -2221,6 +2221,11 @@ void intel_hdcp_update_pipe(struct intel_atomic_state *state, desired_and_not_enabled = hdcp->value != DRM_MODE_CONTENT_PROTECTION_ENABLED; mutex_unlock(&hdcp->mutex); + + if (!desired_and_not_enabled && !content_protection_type_changed) { + drm_connector_get(&connector->base); + schedule_work(&hdcp->prop_work); + } } if (desired_and_not_enabled || content_protection_type_changed)
When crtc state need_modeset is true it is not necessary it is going to be a real modeset, it can turns to be a update_pipe instead of modeset. This turns content protection property to be DESIRED and hdcp update_pipe left with property to be in DESIRED state but actually hdcp->value was ENABLED. This caught with DP MST setup, when disabling HDCP on a connector sets the crtc state need_modeset to true for all crtc driving the other DP-MST topology connectors. v2: Fix WARN_ON(connector->base.registration_state == DRM_CONNECTOR_REGISTERED) Fixes: 33f9a623bfc6 ("drm/i915/hdcp: Update CP as per the kernel internal state") Cc: Ramalingam C <ramalingam.c@intel.com> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com> --- drivers/gpu/drm/i915/display/intel_hdcp.c | 5 +++++ 1 file changed, 5 insertions(+)