| Message ID | Y8K2SS/zNiPAmLsS@ubun2204.myguest.virtualbox.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/display: Convert i9xx_pipe_crc_auto_source to void | expand |
On Sat, Jan 14, 2023 at 07:33:53PM +0530, Deepak R Varma wrote: > Convert function i9xx_pipe_crc_auto_source() to return void instead > of int since the current implementation always returns 0 to the caller. > Issue identified using returnvar Coccinelle semantic patch. could you please share the coccinelle commands and files you used here? > > Signed-off-by: Deepak R Varma <drv@mailo.com> > --- > Please note: The change is compile tested only. np, our CI liked it. I liked the clean up as well: Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > > drivers/gpu/drm/i915/display/intel_pipe_crc.c | 23 ++++++------------- > 1 file changed, 7 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_pipe_crc.c b/drivers/gpu/drm/i915/display/intel_pipe_crc.c > index e9774670e3f6..8d3ea8d7b737 100644 > --- a/drivers/gpu/drm/i915/display/intel_pipe_crc.c > +++ b/drivers/gpu/drm/i915/display/intel_pipe_crc.c > @@ -72,14 +72,13 @@ static int i8xx_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source, > return 0; > } > > -static int i9xx_pipe_crc_auto_source(struct drm_i915_private *dev_priv, > - enum pipe pipe, > - enum intel_pipe_crc_source *source) > +static void i9xx_pipe_crc_auto_source(struct drm_i915_private *dev_priv, > + enum pipe pipe, > + enum intel_pipe_crc_source *source) > { > struct intel_encoder *encoder; > struct intel_crtc *crtc; > struct intel_digital_port *dig_port; > - int ret = 0; > > *source = INTEL_PIPE_CRC_SOURCE_PIPE; > > @@ -121,8 +120,6 @@ static int i9xx_pipe_crc_auto_source(struct drm_i915_private *dev_priv, > } > } > drm_modeset_unlock_all(&dev_priv->drm); > - > - return ret; > } > > static int vlv_pipe_crc_ctl_reg(struct drm_i915_private *dev_priv, > @@ -132,11 +129,8 @@ static int vlv_pipe_crc_ctl_reg(struct drm_i915_private *dev_priv, > { > bool need_stable_symbols = false; > > - if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) { > - int ret = i9xx_pipe_crc_auto_source(dev_priv, pipe, source); > - if (ret) > - return ret; > - } > + if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) > + i9xx_pipe_crc_auto_source(dev_priv, pipe, source); > > switch (*source) { > case INTEL_PIPE_CRC_SOURCE_PIPE: > @@ -200,11 +194,8 @@ static int i9xx_pipe_crc_ctl_reg(struct drm_i915_private *dev_priv, > enum intel_pipe_crc_source *source, > u32 *val) > { > - if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) { > - int ret = i9xx_pipe_crc_auto_source(dev_priv, pipe, source); > - if (ret) > - return ret; > - } > + if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) > + i9xx_pipe_crc_auto_source(dev_priv, pipe, source); > > switch (*source) { > case INTEL_PIPE_CRC_SOURCE_PIPE: > -- > 2.34.1 > > >
On Tue, Jan 17, 2023 at 02:21:59PM -0500, Rodrigo Vivi wrote: > On Sat, Jan 14, 2023 at 07:33:53PM +0530, Deepak R Varma wrote: > > Convert function i9xx_pipe_crc_auto_source() to return void instead > > of int since the current implementation always returns 0 to the caller. > > Issue identified using returnvar Coccinelle semantic patch. > > could you please share the coccinelle commands and files you used here? Hello Rodrigo, I used following command to identify this change opportunity: make coccicheck COCCI=scripts/coccinelle/misc/returnvar.cocci M=drivers/gpu/drm/ Let me know if you would like me to include the same in the commit message. > > > > > Signed-off-by: Deepak R Varma <drv@mailo.com> > > --- > > Please note: The change is compile tested only. > > np, our CI liked it. > > I liked the clean up as well: > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Thank you for your review and the feedback. Regards, ./drv > > > > > > > drivers/gpu/drm/i915/display/intel_pipe_crc.c | 23 ++++++------------- > > 1 file changed, 7 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_pipe_crc.c b/drivers/gpu/drm/i915/display/intel_pipe_crc.c > > index e9774670e3f6..8d3ea8d7b737 100644 > > --- a/drivers/gpu/drm/i915/display/intel_pipe_crc.c > > +++ b/drivers/gpu/drm/i915/display/intel_pipe_crc.c > > @@ -72,14 +72,13 @@ static int i8xx_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source, > > return 0; > > } > > > > -static int i9xx_pipe_crc_auto_source(struct drm_i915_private *dev_priv, > > - enum pipe pipe, > > - enum intel_pipe_crc_source *source) > > +static void i9xx_pipe_crc_auto_source(struct drm_i915_private *dev_priv, > > + enum pipe pipe, > > + enum intel_pipe_crc_source *source) > > { > > struct intel_encoder *encoder; > > struct intel_crtc *crtc; > > struct intel_digital_port *dig_port; > > - int ret = 0; > > > > *source = INTEL_PIPE_CRC_SOURCE_PIPE; > > > > @@ -121,8 +120,6 @@ static int i9xx_pipe_crc_auto_source(struct drm_i915_private *dev_priv, > > } > > } > > drm_modeset_unlock_all(&dev_priv->drm); > > - > > - return ret; > > } > > > > static int vlv_pipe_crc_ctl_reg(struct drm_i915_private *dev_priv, > > @@ -132,11 +129,8 @@ static int vlv_pipe_crc_ctl_reg(struct drm_i915_private *dev_priv, > > { > > bool need_stable_symbols = false; > > > > - if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) { > > - int ret = i9xx_pipe_crc_auto_source(dev_priv, pipe, source); > > - if (ret) > > - return ret; > > - } > > + if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) > > + i9xx_pipe_crc_auto_source(dev_priv, pipe, source); > > > > switch (*source) { > > case INTEL_PIPE_CRC_SOURCE_PIPE: > > @@ -200,11 +194,8 @@ static int i9xx_pipe_crc_ctl_reg(struct drm_i915_private *dev_priv, > > enum intel_pipe_crc_source *source, > > u32 *val) > > { > > - if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) { > > - int ret = i9xx_pipe_crc_auto_source(dev_priv, pipe, source); > > - if (ret) > > - return ret; > > - } > > + if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) > > + i9xx_pipe_crc_auto_source(dev_priv, pipe, source); > > > > switch (*source) { > > case INTEL_PIPE_CRC_SOURCE_PIPE: > > -- > > 2.34.1 > > > > > >
On Wed, Jan 18, 2023 at 10:13:04AM +0530, Deepak R Varma wrote: > On Tue, Jan 17, 2023 at 02:21:59PM -0500, Rodrigo Vivi wrote: > > On Sat, Jan 14, 2023 at 07:33:53PM +0530, Deepak R Varma wrote: > > > Convert function i9xx_pipe_crc_auto_source() to return void instead > > > of int since the current implementation always returns 0 to the caller. > > > Issue identified using returnvar Coccinelle semantic patch. > > > > could you please share the coccinelle commands and files you used here? > > Hello Rodrigo, > I used following command to identify this change opportunity: > > make coccicheck COCCI=scripts/coccinelle/misc/returnvar.cocci M=drivers/gpu/drm/ > > Let me know if you would like me to include the same in the commit message. Thanks for the line. Added to the commit message while pushing it. Thanks, Rodrigo. > > > > > > > > > Signed-off-by: Deepak R Varma <drv@mailo.com> > > > --- > > > Please note: The change is compile tested only. > > > > np, our CI liked it. > > > > I liked the clean up as well: > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Thank you for your review and the feedback. > > Regards, > ./drv > > > > > > > > > > > > drivers/gpu/drm/i915/display/intel_pipe_crc.c | 23 ++++++------------- > > > 1 file changed, 7 insertions(+), 16 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_pipe_crc.c b/drivers/gpu/drm/i915/display/intel_pipe_crc.c > > > index e9774670e3f6..8d3ea8d7b737 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_pipe_crc.c > > > +++ b/drivers/gpu/drm/i915/display/intel_pipe_crc.c > > > @@ -72,14 +72,13 @@ static int i8xx_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source, > > > return 0; > > > } > > > > > > -static int i9xx_pipe_crc_auto_source(struct drm_i915_private *dev_priv, > > > - enum pipe pipe, > > > - enum intel_pipe_crc_source *source) > > > +static void i9xx_pipe_crc_auto_source(struct drm_i915_private *dev_priv, > > > + enum pipe pipe, > > > + enum intel_pipe_crc_source *source) > > > { > > > struct intel_encoder *encoder; > > > struct intel_crtc *crtc; > > > struct intel_digital_port *dig_port; > > > - int ret = 0; > > > > > > *source = INTEL_PIPE_CRC_SOURCE_PIPE; > > > > > > @@ -121,8 +120,6 @@ static int i9xx_pipe_crc_auto_source(struct drm_i915_private *dev_priv, > > > } > > > } > > > drm_modeset_unlock_all(&dev_priv->drm); > > > - > > > - return ret; > > > } > > > > > > static int vlv_pipe_crc_ctl_reg(struct drm_i915_private *dev_priv, > > > @@ -132,11 +129,8 @@ static int vlv_pipe_crc_ctl_reg(struct drm_i915_private *dev_priv, > > > { > > > bool need_stable_symbols = false; > > > > > > - if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) { > > > - int ret = i9xx_pipe_crc_auto_source(dev_priv, pipe, source); > > > - if (ret) > > > - return ret; > > > - } > > > + if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) > > > + i9xx_pipe_crc_auto_source(dev_priv, pipe, source); > > > > > > switch (*source) { > > > case INTEL_PIPE_CRC_SOURCE_PIPE: > > > @@ -200,11 +194,8 @@ static int i9xx_pipe_crc_ctl_reg(struct drm_i915_private *dev_priv, > > > enum intel_pipe_crc_source *source, > > > u32 *val) > > > { > > > - if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) { > > > - int ret = i9xx_pipe_crc_auto_source(dev_priv, pipe, source); > > > - if (ret) > > > - return ret; > > > - } > > > + if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) > > > + i9xx_pipe_crc_auto_source(dev_priv, pipe, source); > > > > > > switch (*source) { > > > case INTEL_PIPE_CRC_SOURCE_PIPE: > > > -- > > > 2.34.1 > > > > > > > > > > >
diff --git a/drivers/gpu/drm/i915/display/intel_pipe_crc.c b/drivers/gpu/drm/i915/display/intel_pipe_crc.c index e9774670e3f6..8d3ea8d7b737 100644 --- a/drivers/gpu/drm/i915/display/intel_pipe_crc.c +++ b/drivers/gpu/drm/i915/display/intel_pipe_crc.c @@ -72,14 +72,13 @@ static int i8xx_pipe_crc_ctl_reg(enum intel_pipe_crc_source *source, return 0; } -static int i9xx_pipe_crc_auto_source(struct drm_i915_private *dev_priv, - enum pipe pipe, - enum intel_pipe_crc_source *source) +static void i9xx_pipe_crc_auto_source(struct drm_i915_private *dev_priv, + enum pipe pipe, + enum intel_pipe_crc_source *source) { struct intel_encoder *encoder; struct intel_crtc *crtc; struct intel_digital_port *dig_port; - int ret = 0; *source = INTEL_PIPE_CRC_SOURCE_PIPE; @@ -121,8 +120,6 @@ static int i9xx_pipe_crc_auto_source(struct drm_i915_private *dev_priv, } } drm_modeset_unlock_all(&dev_priv->drm); - - return ret; } static int vlv_pipe_crc_ctl_reg(struct drm_i915_private *dev_priv, @@ -132,11 +129,8 @@ static int vlv_pipe_crc_ctl_reg(struct drm_i915_private *dev_priv, { bool need_stable_symbols = false; - if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) { - int ret = i9xx_pipe_crc_auto_source(dev_priv, pipe, source); - if (ret) - return ret; - } + if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) + i9xx_pipe_crc_auto_source(dev_priv, pipe, source); switch (*source) { case INTEL_PIPE_CRC_SOURCE_PIPE: @@ -200,11 +194,8 @@ static int i9xx_pipe_crc_ctl_reg(struct drm_i915_private *dev_priv, enum intel_pipe_crc_source *source, u32 *val) { - if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) { - int ret = i9xx_pipe_crc_auto_source(dev_priv, pipe, source); - if (ret) - return ret; - } + if (*source == INTEL_PIPE_CRC_SOURCE_AUTO) + i9xx_pipe_crc_auto_source(dev_priv, pipe, source); switch (*source) { case INTEL_PIPE_CRC_SOURCE_PIPE:
Convert function i9xx_pipe_crc_auto_source() to return void instead of int since the current implementation always returns 0 to the caller. Issue identified using returnvar Coccinelle semantic patch. Signed-off-by: Deepak R Varma <drv@mailo.com> --- Please note: The change is compile tested only. drivers/gpu/drm/i915/display/intel_pipe_crc.c | 23 ++++++------------- 1 file changed, 7 insertions(+), 16 deletions(-)