2023-01-14 14:18:02

by Deepak R Varma

Subject: [PATCH] drm/i915/display: Convert i9xx_pipe_crc_auto_source to void

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 <[email protected]>
---
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(-)

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

2023-01-17 21:05:24

by Rodrigo Vivi

Subject: Re: [PATCH] drm/i915/display: Convert i9xx_pipe_crc_auto_source to void

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 <[email protected]>
> ---
> Please note: The change is compile tested only.

np, our CI liked it.

I liked the clean up as well:
Reviewed-by: Rodrigo Vivi <[email protected]>

>
>
> 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
>
>
>

2023-01-18 05:22:43

by Deepak R Varma

Subject: Re: [PATCH] drm/i915/display: Convert i9xx_pipe_crc_auto_source to void

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 <[email protected]>
> > ---
> > Please note: The change is compile tested only.
>
> np, our CI liked it.
>
> I liked the clean up as well:
> Reviewed-by: Rodrigo Vivi <[email protected]>

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
> >
> >
> >

2023-01-18 17:22:51

by Rodrigo Vivi

Subject: Re: [PATCH] drm/i915/display: Convert i9xx_pipe_crc_auto_source to void

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 <[email protected]>
> > > ---
> > > Please note: The change is compile tested only.
> >
> > np, our CI liked it.
> >
> > I liked the clean up as well:
> > Reviewed-by: Rodrigo Vivi <[email protected]>
>
> 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
> > >
> > >
> > >
>
>