| Message ID | 20240507035037.1025012-3-suraj.kandpal@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Fixes in hdcp remote capability | expand |
On Tue, May 07, 2024 at 09:20:37AM +0530, Suraj Kandpal wrote: > Check mst_port field in intel_connector to check connector type > rather than rely on encoder as it may not be attached to connector > at times. > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > --- > drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c > index 551c862ed7a6..2edffe62f360 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c > @@ -693,7 +693,7 @@ int intel_dp_hdcp_get_remote_capability(struct intel_connector *connector, > > *hdcp_capable = false; > *hdcp2_capable = false; > - if (!intel_encoder_is_mst(connector->encoder)) > + if (!connector->mst_port) I suppose this fixes https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/10898 Could you add the Closes: line for it? Can this function be called for anything else than an MST connector? Afaics it's only called from intel_connector_info() -> intel_hdcp_info(..., remote_req = true) only for MST connectors, which makes sense since only MST connectors would have remote caps. In that case it would be enough to simply remove the encoder check which leads to the NULL deref in case the output is disabled. > return -EINVAL; > > aux = &connector->port->aux; > -- > 2.43.2 >
> -----Original Message----- > From: Deak, Imre <imre.deak@intel.com> > Sent: Friday, May 17, 2024 6:19 PM > To: Kandpal, Suraj <suraj.kandpal@intel.com> > Cc: intel-gfx@lists.freedesktop.org; Borah, Chaitanya Kumar > <chaitanya.kumar.borah@intel.com>; Shankar, Uma > <uma.shankar@intel.com>; Nautiyal, Ankit K <ankit.k.nautiyal@intel.com> > Subject: Re: [PATCH 2/2] drm/i915/hdcp: Check mst_port to determine > connector type > > On Tue, May 07, 2024 at 09:20:37AM +0530, Suraj Kandpal wrote: > > Check mst_port field in intel_connector to check connector type rather > > than rely on encoder as it may not be attached to connector at times. > > > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c > > b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c > > index 551c862ed7a6..2edffe62f360 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c > > @@ -693,7 +693,7 @@ int intel_dp_hdcp_get_remote_capability(struct > > intel_connector *connector, > > > > *hdcp_capable = false; > > *hdcp2_capable = false; > > - if (!intel_encoder_is_mst(connector->encoder)) > > + if (!connector->mst_port) > > I suppose this fixes > https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/10898 > > Could you add the Closes: line for it? > Sure Imre will add that. > Can this function be called for anything else than an MST connector? > > Afaics it's only called from > > intel_connector_info() -> > intel_hdcp_info(..., remote_req = true) > > only for MST connectors, which makes sense since only MST connectors would > have remote caps. In that case it would be enough to simply remove the > encoder check which leads to the NULL deref in case the output is disabled. > Right this function is not invoked from anywhere but hdcp_info() since this was Created just to have a sense of the actual HDCP capability of remote monitor rather than having to display the first monitor's HDCP capability and repeating it for the second monitor specially when in daisy chain setup. Regards, Suraj Kandpal > > return -EINVAL; > > > > aux = &connector->port->aux; > > -- > > 2.43.2 > >
On Mon, May 20, 2024 at 06:58:19AM +0300, Kandpal, Suraj wrote: > > > > -----Original Message----- > > From: Deak, Imre <imre.deak@intel.com> > > Sent: Friday, May 17, 2024 6:19 PM > > To: Kandpal, Suraj <suraj.kandpal@intel.com> > > Cc: intel-gfx@lists.freedesktop.org; Borah, Chaitanya Kumar > > <chaitanya.kumar.borah@intel.com>; Shankar, Uma > > <uma.shankar@intel.com>; Nautiyal, Ankit K <ankit.k.nautiyal@intel.com> > > Subject: Re: [PATCH 2/2] drm/i915/hdcp: Check mst_port to determine > > connector type > > > > On Tue, May 07, 2024 at 09:20:37AM +0530, Suraj Kandpal wrote: > > > Check mst_port field in intel_connector to check connector type rather > > > than rely on encoder as it may not be attached to connector at times. > > > > > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> > > > --- > > > drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c > > > b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c > > > index 551c862ed7a6..2edffe62f360 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c > > > +++ b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c > > > @@ -693,7 +693,7 @@ int intel_dp_hdcp_get_remote_capability(struct > > > intel_connector *connector, > > > > > > *hdcp_capable = false; > > > *hdcp2_capable = false; > > > - if (!intel_encoder_is_mst(connector->encoder)) > > > + if (!connector->mst_port) > > > > I suppose this fixes > > https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/10898 > > > > Could you add the Closes: line for it? > > > > Sure Imre will add that. > > > Can this function be called for anything else than an MST connector? > > > > Afaics it's only called from > > > > intel_connector_info() -> > > intel_hdcp_info(..., remote_req = true) > > > > only for MST connectors, which makes sense since only MST connectors would > > have remote caps. In that case it would be enough to simply remove the > > encoder check which leads to the NULL deref in case the output is disabled. > > Right this function is not invoked from anywhere but hdcp_info() since > this was Created just to have a sense of the actual HDCP capability of > remote monitor rather than having to display the first monitor's HDCP > capability and repeating it for the second monitor specially when in > daisy chain setup. I meant that it's strange to check connector->mst_port in intel_connector_info() and then check the same condition again in the above function, which is always true and thus redundant. In any case the change does fix the issue, so on the patchset: Reviewed-by: Imre Deak <imre.deak@intel.com> > Regards, > Suraj Kandpal > > > > return -EINVAL; > > > > > > aux = &connector->port->aux; > > > -- > > > 2.43.2 > > >
diff --git a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c index 551c862ed7a6..2edffe62f360 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_dp_hdcp.c @@ -693,7 +693,7 @@ int intel_dp_hdcp_get_remote_capability(struct intel_connector *connector, *hdcp_capable = false; *hdcp2_capable = false; - if (!intel_encoder_is_mst(connector->encoder)) + if (!connector->mst_port) return -EINVAL; aux = &connector->port->aux;
Check mst_port field in intel_connector to check connector type rather than rely on encoder as it may not be attached to connector at times. Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com> --- drivers/gpu/drm/i915/display/intel_dp_hdcp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)