| Message ID | 20210115092034.8804-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/display: Set error value before returning in intel_hdcp*_disable() | expand |
On Fri, 15 Jan 2021, Chris Wilson <chris@chris-wilson.co.uk> wrote: > drivers/gpu/drm/i915/display/intel_hdcp.c:886 _intel_hdcp_disable() error: uninitialized symbol 'ret'. > drivers/gpu/drm/i915/display/intel_hdcp.c:1951 _intel_hdcp2_disable() error: uninitialized symbol 'ret'. > > Given the comment that this error reflects the status of all connectors > within this MST topology, I opted to use -EBUSY to indicate not all were > ready. > > Fixes: 2a743b7b8a8b ("drm/i915/hdcp: Configure HDCP1.4 MST steram encryption status") > Fixes: 899c8762f981 ("drm/i915/hdcp: Configure HDCP2.2 MST steram encryption status") > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Ramalingam C <ramalingam.c@intel.com> > Cc: Uma Shankar <uma.shankar@intel.com> > Cc: Anshuman Gupta <anshuman.gupta@intel.com> > Cc: Sean Paul <seanpaul@chromium.org> > Cc: Jani Nikula <jani.nikula@intel.com> Reviewed-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/display/intel_hdcp.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c > index db8dff2eeb0a..2be4822b91a0 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > @@ -883,7 +883,7 @@ static int _intel_hdcp_disable(struct intel_connector *connector) > * until it disabled HDCP encryption for all connectors in MST topology. > */ > if (dig_port->num_hdcp_streams > 0) > - return ret; > + return -EBUSY; > > hdcp->hdcp_encrypted = false; > intel_de_write(dev_priv, HDCP_CONF(dev_priv, cpu_transcoder, port), 0); > @@ -1948,7 +1948,7 @@ static int _intel_hdcp2_disable(struct intel_connector *connector) > } > > if (dig_port->num_hdcp_streams > 0) > - return ret; > + return -EBUSY; > > ret = hdcp2_disable_encryption(connector);
> -----Original Message----- > From: Jani Nikula <jani.nikula@intel.com> > Sent: Friday, January 15, 2021 4:05 PM > To: Chris Wilson <chris@chris-wilson.co.uk>; intel-gfx@lists.freedesktop.org > Cc: Chris Wilson <chris@chris-wilson.co.uk>; C, Ramalingam > <ramalingam.c@intel.com>; Shankar, Uma <uma.shankar@intel.com>; > Gupta, Anshuman <anshuman.gupta@intel.com>; Sean Paul > <seanpaul@chromium.org> > Subject: Re: [PATCH] drm/i915/display: Set error value before returning in > intel_hdcp*_disable() > > On Fri, 15 Jan 2021, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > drivers/gpu/drm/i915/display/intel_hdcp.c:886 _intel_hdcp_disable() > error: uninitialized symbol 'ret'. > > drivers/gpu/drm/i915/display/intel_hdcp.c:1951 _intel_hdcp2_disable() > error: uninitialized symbol 'ret'. > > > > Given the comment that this error reflects the status of all > > connectors within this MST topology, I opted to use -EBUSY to indicate > > not all were ready. > > > > Fixes: 2a743b7b8a8b ("drm/i915/hdcp: Configure HDCP1.4 MST steram > > encryption status") > > Fixes: 899c8762f981 ("drm/i915/hdcp: Configure HDCP2.2 MST steram > > encryption status") > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Ramalingam C <ramalingam.c@intel.com> > > Cc: Uma Shankar <uma.shankar@intel.com> > > Cc: Anshuman Gupta <anshuman.gupta@intel.com> > > Cc: Sean Paul <seanpaul@chromium.org> > > Cc: Jani Nikula <jani.nikula@intel.com> > > Reviewed-by: Jani Nikula <jani.nikula@intel.com> > > > --- > > drivers/gpu/drm/i915/display/intel_hdcp.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c > > b/drivers/gpu/drm/i915/display/intel_hdcp.c > > index db8dff2eeb0a..2be4822b91a0 100644 > > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > > @@ -883,7 +883,7 @@ static int _intel_hdcp_disable(struct > intel_connector *connector) > > * until it disabled HDCP encryption for all connectors in MST > topology. > > */ > > if (dig_port->num_hdcp_streams > 0) > > - return ret; > > + return -EBUSY; Please return 0 as a fix to static analysis warning, It is not an error. Above condition will only true for MST case, so it always was returning return value from stream_encryption(). Thanks, Anshuman Gupta. > > > > hdcp->hdcp_encrypted = false; > > intel_de_write(dev_priv, HDCP_CONF(dev_priv, cpu_transcoder, > port), > > 0); @@ -1948,7 +1948,7 @@ static int _intel_hdcp2_disable(struct > intel_connector *connector) > > } > > > > if (dig_port->num_hdcp_streams > 0) > > - return ret; > > + return -EBUSY; > > > > ret = hdcp2_disable_encryption(connector); > > -- > Jani Nikula, Intel Open Source Graphics Center
> -----Original Message----- > From: Gupta, Anshuman > Sent: Friday, January 15, 2021 5:39 PM > To: 'Jani Nikula' <jani.nikula@intel.com>; Chris Wilson <chris@chris- > wilson.co.uk>; intel-gfx@lists.freedesktop.org > Cc: Chris Wilson <chris@chris-wilson.co.uk>; C, Ramalingam > <ramalingam.c@intel.com>; Shankar, Uma <uma.shankar@intel.com>; > Sean Paul <seanpaul@chromium.org> > Subject: RE: [PATCH] drm/i915/display: Set error value before returning in > intel_hdcp*_disable() > > > > > -----Original Message----- > > From: Jani Nikula <jani.nikula@intel.com> > > Sent: Friday, January 15, 2021 4:05 PM > > To: Chris Wilson <chris@chris-wilson.co.uk>; > > intel-gfx@lists.freedesktop.org > > Cc: Chris Wilson <chris@chris-wilson.co.uk>; C, Ramalingam > > <ramalingam.c@intel.com>; Shankar, Uma <uma.shankar@intel.com>; > Gupta, > > Anshuman <anshuman.gupta@intel.com>; Sean Paul > <seanpaul@chromium.org> > > Subject: Re: [PATCH] drm/i915/display: Set error value before > > returning in > > intel_hdcp*_disable() > > > > On Fri, 15 Jan 2021, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > drivers/gpu/drm/i915/display/intel_hdcp.c:886 _intel_hdcp_disable() > > error: uninitialized symbol 'ret'. > > > drivers/gpu/drm/i915/display/intel_hdcp.c:1951 > > > _intel_hdcp2_disable() > > error: uninitialized symbol 'ret'. > > > > > > Given the comment that this error reflects the status of all > > > connectors within this MST topology, I opted to use -EBUSY to > > > indicate not all were ready. > > > > > > Fixes: 2a743b7b8a8b ("drm/i915/hdcp: Configure HDCP1.4 MST steram > > > encryption status") > > > Fixes: 899c8762f981 ("drm/i915/hdcp: Configure HDCP2.2 MST steram > > > encryption status") > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > > Cc: Ramalingam C <ramalingam.c@intel.com> > > > Cc: Uma Shankar <uma.shankar@intel.com> > > > Cc: Anshuman Gupta <anshuman.gupta@intel.com> > > > Cc: Sean Paul <seanpaul@chromium.org> > > > Cc: Jani Nikula <jani.nikula@intel.com> > > > > Reviewed-by: Jani Nikula <jani.nikula@intel.com> > > > > > --- > > > drivers/gpu/drm/i915/display/intel_hdcp.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c > > > b/drivers/gpu/drm/i915/display/intel_hdcp.c > > > index db8dff2eeb0a..2be4822b91a0 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > > > @@ -883,7 +883,7 @@ static int _intel_hdcp_disable(struct > > intel_connector *connector) > > > * until it disabled HDCP encryption for all connectors in MST > > topology. > > > */ > > > if (dig_port->num_hdcp_streams > 0) > > > - return ret; > > > + return -EBUSY; > Please return 0 as a fix to static analysis warning, It is not an error. > Above condition will only true for MST case, so it always was returning > return value from stream_encryption(). > > Thanks, > Anshuman Gupta. Floated a patch to fix this https://patchwork.freedesktop.org/patch/414427/?series=85917&rev=1 Thanks, Anshuman Gupta. > > > > > > hdcp->hdcp_encrypted = false; > > > intel_de_write(dev_priv, HDCP_CONF(dev_priv, cpu_transcoder, > > port), > > > 0); @@ -1948,7 +1948,7 @@ static int _intel_hdcp2_disable(struct > > intel_connector *connector) > > > } > > > > > > if (dig_port->num_hdcp_streams > 0) > > > - return ret; > > > + return -EBUSY; > > > > > > ret = hdcp2_disable_encryption(connector); > > > > -- > > Jani Nikula, Intel Open Source Graphics Center
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c index db8dff2eeb0a..2be4822b91a0 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c @@ -883,7 +883,7 @@ static int _intel_hdcp_disable(struct intel_connector *connector) * until it disabled HDCP encryption for all connectors in MST topology. */ if (dig_port->num_hdcp_streams > 0) - return ret; + return -EBUSY; hdcp->hdcp_encrypted = false; intel_de_write(dev_priv, HDCP_CONF(dev_priv, cpu_transcoder, port), 0); @@ -1948,7 +1948,7 @@ static int _intel_hdcp2_disable(struct intel_connector *connector) } if (dig_port->num_hdcp_streams > 0) - return ret; + return -EBUSY; ret = hdcp2_disable_encryption(connector);
drivers/gpu/drm/i915/display/intel_hdcp.c:886 _intel_hdcp_disable() error: uninitialized symbol 'ret'. drivers/gpu/drm/i915/display/intel_hdcp.c:1951 _intel_hdcp2_disable() error: uninitialized symbol 'ret'. Given the comment that this error reflects the status of all connectors within this MST topology, I opted to use -EBUSY to indicate not all were ready. Fixes: 2a743b7b8a8b ("drm/i915/hdcp: Configure HDCP1.4 MST steram encryption status") Fixes: 899c8762f981 ("drm/i915/hdcp: Configure HDCP2.2 MST steram encryption status") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Ramalingam C <ramalingam.c@intel.com> Cc: Uma Shankar <uma.shankar@intel.com> Cc: Anshuman Gupta <anshuman.gupta@intel.com> Cc: Sean Paul <seanpaul@chromium.org> Cc: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/display/intel_hdcp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)