Re: [PATCH 03/19] gpu: drm: imx: ipuv3-plane: Mark 'crtc_state' as __always_unused
On 11/6/20 8:41 AM, Lee Jones wrote: > On Thu, 05 Nov 2020, Ahmad Fatoum wrote: > >> Hello Lee, >> >> On 11/5/20 3:45 PM, Lee Jones wrote: >>> In the macro for_each_oldnew_crtc_in_state() 'crtc_state' is provided >>> as a container for state->crtcs[i].new_state, but is not utilised in >>> this use-case. We cannot simply delete the variable, so here we tell >>> the compiler that we're intentionally discarding the read value. >> >> for_each_oldnew_crtc_in_state already (void) casts the drm_crtc and the old >> drm_crtc_state to silence unused-but-set-variable warning. Should we maybe >> (void) cast the new crtc_state as well? > > From what I saw, it only void casts the ones which aren't assigned. How do you mean? I wonder if #define for_each_oldnew_crtc_in_state(__state, crtc, old_crtc_state, new_crtc_state, __i) \ for ((__i) = 0; \ (__i) < (__state)->dev->mode_config.num_crtc; \ (__i)++) \ for_each_if ((__state)->crtcs[__i].ptr && \ ((crtc) = (__state)->crtcs[__i].ptr, \ (void)(crtc) /* Only to avoid unused-but-set-variable warning */, \ (old_crtc_state) = (__state)->crtcs[__i].old_state, \ (void)(old_crtc_state) /* Only to avoid unused-but-set-variable warning */, \ -(new_crtc_state) = (__state)->crtcs[__i].new_state, 1)) +(new_crtc_state) = (__state)->crtcs[__i].new_state, \ +(void)(new_crtc_state), 1)) wouldn't be better. > >>> Fixes the following W=1 kernel build warning(s): >>> >>> drivers/gpu/drm/imx/ipuv3-plane.c: In function ‘ipu_planes_assign_pre’: >>> drivers/gpu/drm/imx/ipuv3-plane.c:746:42: warning: variable ‘crtc_state’ >>> set but not used [-Wunused-but-set-variable] >>> >>> Cc: Philipp Zabel >>> Cc: David Airlie >>> Cc: Daniel Vetter >>> Cc: Shawn Guo >>> Cc: Sascha Hauer >>> Cc: Pengutronix Kernel Team >>> Cc: Fabio Estevam >>> Cc: NXP Linux Team >>> Cc: dri-devel@lists.freedesktop.org >>> Signed-off-by: Lee Jones >>> --- >>> drivers/gpu/drm/imx/ipuv3-plane.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c >>> b/drivers/gpu/drm/imx/ipuv3-plane.c >>> index 8a4235d9d9f1e..acc0a3ce4992f 100644 >>> --- a/drivers/gpu/drm/imx/ipuv3-plane.c >>> +++ b/drivers/gpu/drm/imx/ipuv3-plane.c >>> @@ -743,7 +743,7 @@ bool ipu_plane_atomic_update_pending(struct drm_plane >>> *plane) >>> int ipu_planes_assign_pre(struct drm_device *dev, >>> struct drm_atomic_state *state) >>> { >>> - struct drm_crtc_state *old_crtc_state, *crtc_state; >>> + struct drm_crtc_state *old_crtc_state, __always_unused *crtc_state; >>> struct drm_plane_state *plane_state; >>> struct ipu_plane_state *ipu_state; >>> struct ipu_plane *ipu_plane; >>> >> > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 03/19] gpu: drm: imx: ipuv3-plane: Mark 'crtc_state' as __always_unused
On Fri, 06 Nov 2020, Ahmad Fatoum wrote: > On 11/6/20 8:41 AM, Lee Jones wrote: > > On Thu, 05 Nov 2020, Ahmad Fatoum wrote: > > > >> Hello Lee, > >> > >> On 11/5/20 3:45 PM, Lee Jones wrote: > >>> In the macro for_each_oldnew_crtc_in_state() 'crtc_state' is provided > >>> as a container for state->crtcs[i].new_state, but is not utilised in > >>> this use-case. We cannot simply delete the variable, so here we tell > >>> the compiler that we're intentionally discarding the read value. > >> > >> for_each_oldnew_crtc_in_state already (void) casts the drm_crtc and the old > >> drm_crtc_state to silence unused-but-set-variable warning. Should we maybe > >> (void) cast the new crtc_state as well? > > > > From what I saw, it only void casts the ones which aren't assigned. > > How do you mean? I wonder if > > #define for_each_oldnew_crtc_in_state(__state, crtc, old_crtc_state, > new_crtc_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_crtc; \ > (__i)++) \ > for_each_if ((__state)->crtcs[__i].ptr && \ > ((crtc) = (__state)->crtcs[__i].ptr, \ > (void)(crtc) /* Only to avoid > unused-but-set-variable warning */, \ > (old_crtc_state) = > (__state)->crtcs[__i].old_state, \ > (void)(old_crtc_state) /* Only to avoid > unused-but-set-variable warning */, \ > -(new_crtc_state) = > (__state)->crtcs[__i].new_state, 1)) > +(new_crtc_state) = > (__state)->crtcs[__i].new_state, \ > +(void)(new_crtc_state), 1)) > > wouldn't be better. That also works for me. I can fix this up. -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 03/19] gpu: drm: imx: ipuv3-plane: Mark 'crtc_state' as __always_unused
Hello Lee, On 11/5/20 3:45 PM, Lee Jones wrote: > In the macro for_each_oldnew_crtc_in_state() 'crtc_state' is provided > as a container for state->crtcs[i].new_state, but is not utilised in > this use-case. We cannot simply delete the variable, so here we tell > the compiler that we're intentionally discarding the read value. for_each_oldnew_crtc_in_state already (void) casts the drm_crtc and the old drm_crtc_state to silence unused-but-set-variable warning. Should we maybe (void) cast the new crtc_state as well? Cheers Ahmad > > Fixes the following W=1 kernel build warning(s): > > drivers/gpu/drm/imx/ipuv3-plane.c: In function ‘ipu_planes_assign_pre’: > drivers/gpu/drm/imx/ipuv3-plane.c:746:42: warning: variable ‘crtc_state’ set > but not used [-Wunused-but-set-variable] > > Cc: Philipp Zabel > Cc: David Airlie > Cc: Daniel Vetter > Cc: Shawn Guo > Cc: Sascha Hauer > Cc: Pengutronix Kernel Team > Cc: Fabio Estevam > Cc: NXP Linux Team > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Lee Jones > --- > drivers/gpu/drm/imx/ipuv3-plane.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c > b/drivers/gpu/drm/imx/ipuv3-plane.c > index 8a4235d9d9f1e..acc0a3ce4992f 100644 > --- a/drivers/gpu/drm/imx/ipuv3-plane.c > +++ b/drivers/gpu/drm/imx/ipuv3-plane.c > @@ -743,7 +743,7 @@ bool ipu_plane_atomic_update_pending(struct drm_plane > *plane) > int ipu_planes_assign_pre(struct drm_device *dev, > struct drm_atomic_state *state) > { > - struct drm_crtc_state *old_crtc_state, *crtc_state; > + struct drm_crtc_state *old_crtc_state, __always_unused *crtc_state; > struct drm_plane_state *plane_state; > struct ipu_plane_state *ipu_state; > struct ipu_plane *ipu_plane; > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 03/19] gpu: drm: imx: ipuv3-plane: Mark 'crtc_state' as __always_unused
On Thu, 05 Nov 2020, Ahmad Fatoum wrote: > Hello Lee, > > On 11/5/20 3:45 PM, Lee Jones wrote: > > In the macro for_each_oldnew_crtc_in_state() 'crtc_state' is provided > > as a container for state->crtcs[i].new_state, but is not utilised in > > this use-case. We cannot simply delete the variable, so here we tell > > the compiler that we're intentionally discarding the read value. > > for_each_oldnew_crtc_in_state already (void) casts the drm_crtc and the old > drm_crtc_state to silence unused-but-set-variable warning. Should we maybe > (void) cast the new crtc_state as well? From what I saw, it only void casts the ones which aren't assigned. > > Fixes the following W=1 kernel build warning(s): > > > > drivers/gpu/drm/imx/ipuv3-plane.c: In function ‘ipu_planes_assign_pre’: > > drivers/gpu/drm/imx/ipuv3-plane.c:746:42: warning: variable ‘crtc_state’ > > set but not used [-Wunused-but-set-variable] > > > > Cc: Philipp Zabel > > Cc: David Airlie > > Cc: Daniel Vetter > > Cc: Shawn Guo > > Cc: Sascha Hauer > > Cc: Pengutronix Kernel Team > > Cc: Fabio Estevam > > Cc: NXP Linux Team > > Cc: dri-devel@lists.freedesktop.org > > Signed-off-by: Lee Jones > > --- > > drivers/gpu/drm/imx/ipuv3-plane.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c > > b/drivers/gpu/drm/imx/ipuv3-plane.c > > index 8a4235d9d9f1e..acc0a3ce4992f 100644 > > --- a/drivers/gpu/drm/imx/ipuv3-plane.c > > +++ b/drivers/gpu/drm/imx/ipuv3-plane.c > > @@ -743,7 +743,7 @@ bool ipu_plane_atomic_update_pending(struct drm_plane > > *plane) > > int ipu_planes_assign_pre(struct drm_device *dev, > > struct drm_atomic_state *state) > > { > > - struct drm_crtc_state *old_crtc_state, *crtc_state; > > + struct drm_crtc_state *old_crtc_state, __always_unused *crtc_state; > > struct drm_plane_state *plane_state; > > struct ipu_plane_state *ipu_state; > > struct ipu_plane *ipu_plane; > > > -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 03/19] gpu: drm: imx: ipuv3-plane: Mark 'crtc_state' as __always_unused
In the macro for_each_oldnew_crtc_in_state() 'crtc_state' is provided as a container for state->crtcs[i].new_state, but is not utilised in this use-case. We cannot simply delete the variable, so here we tell the compiler that we're intentionally discarding the read value. Fixes the following W=1 kernel build warning(s): drivers/gpu/drm/imx/ipuv3-plane.c: In function ‘ipu_planes_assign_pre’: drivers/gpu/drm/imx/ipuv3-plane.c:746:42: warning: variable ‘crtc_state’ set but not used [-Wunused-but-set-variable] Cc: Philipp Zabel Cc: David Airlie Cc: Daniel Vetter Cc: Shawn Guo Cc: Sascha Hauer Cc: Pengutronix Kernel Team Cc: Fabio Estevam Cc: NXP Linux Team Cc: dri-devel@lists.freedesktop.org Signed-off-by: Lee Jones --- drivers/gpu/drm/imx/ipuv3-plane.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/imx/ipuv3-plane.c b/drivers/gpu/drm/imx/ipuv3-plane.c index 8a4235d9d9f1e..acc0a3ce4992f 100644 --- a/drivers/gpu/drm/imx/ipuv3-plane.c +++ b/drivers/gpu/drm/imx/ipuv3-plane.c @@ -743,7 +743,7 @@ bool ipu_plane_atomic_update_pending(struct drm_plane *plane) int ipu_planes_assign_pre(struct drm_device *dev, struct drm_atomic_state *state) { - struct drm_crtc_state *old_crtc_state, *crtc_state; + struct drm_crtc_state *old_crtc_state, __always_unused *crtc_state; struct drm_plane_state *plane_state; struct ipu_plane_state *ipu_state; struct ipu_plane *ipu_plane; -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel