Re: [PATCH 05/10] drm: Use the state pointer directly in planes atomic_check

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Maxime Ripard <maxime@cerno.tech>
Cc: "Haneen Mohammed" <hamohammed.sa@gmail.com>,
	"Alexandre Belloni" <alexandre.belloni@bootlin.com>,
	"Heiko Stübner" <heiko@sntech.de>,
	"Neil Armstrong" <narmstrong@baylibre.com>,
	"David Airlie" <airlied@linux.ie>,
	"Liviu Dudau" <liviu.dudau@arm.com>,
	dri-devel@lists.freedesktop.org,
	"Sandy Huang" <hjc@rock-chips.com>,
	"Paul Cercueil" <paul@crapouillou.net>,
	"Rob Clark" <robdclark@gmail.com>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Benjamin Gaignard" <benjamin.gaignard@linaro.org>,
	"Anitha Chrisanthus" <anitha.chrisanthus@intel.com>,
	"Daniel Vetter" <daniel.vetter@intel.com>,
	"Mihail Atanassov" <mihail.atanassov@arm.com>,
	"Sam Ravnborg" <sam@ravnborg.org>,
	"Michal Simek" <michal.simek@xilinx.com>,
	linux-kernel@vger.kernel.org,
	"Jerome Brunet" <jbrunet@baylibre.com>,
	"Marek Vasut" <marex@denx.de>,
	linux-rockchip@lists.infradead.org,
	"Fabio Estevam" <festevam@gmail.com>,
	linux-mediatek@lists.infradead.org,
	"Russell King" <linux@armlinux.org.uk>,
	amd-gfx@lists.freedesktop.org,
	"Roland Scheidegger" <sroland@vmware.com>,
	"Xinliang Liu" <xinliang.liu@linaro.org>,
	"Ludovic Desroches" <ludovic.desroches@microchip.com>,
	"James (Qian) Wang" <james.qian.wang@arm.com>,
	"NXP Linux Team" <linux-imx@nxp.com>,
	linux-arm-msm@vger.kernel.org, "Dave Airlie" <airlied@redhat.com>,
	"Xinwei Kong" <kong.kongxinwei@hisilicon.com>,
	"Harry Wentland" <harry.wentland@amd.com>,
	"Hyun Kwon" <hyun.kwon@xilinx.com>,
	"Philipp Zabel" <p.zabel@pengutronix.de>,
	"Chun-Kuang Hu" <chunkuang.hu@kernel.org>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"Leo Li" <sunpeng.li@amd.com>,
	"Chen Feng" <puck.chen@hisilicon.com>,
	"Sascha Hauer" <s.hauer@pengutronix.de>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Martin Blumenstingl" <martin.blumenstingl@googlemail.com>,
	"Hans de Goede" <hdegoede@redhat.com>,
	"John Stultz" <john.stultz@linaro.org>,
	"Stefan Agner" <stefan@agner.ch>,
	"Laurentiu Palcu" <laurentiu.palcu@oss.nxp.com>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	linux-amlogic@lists.infradead.org, "Chen-Yu Tsai" <wens@csie.org>,
	"Sean Paul" <sean@poorly.run>,
	"Pengutronix Kernel Team" <kernel@pengutronix.de>,
	linux-arm-kernel@lists.infradead.org,
	"Melissa Wen" <melissa.srw@gmail.com>,
	"Jernej Skrabec" <jernej.skrabec@siol.net>,
	"VMware Graphics" <linux-graphics-maintainer@vmware.com>,
	"Rodrigo Siqueira" <rodrigosiqueiramelo@gmail.com>,
	"Tomi Valkeinen" <tomba@kernel.org>,
	"Boris Brezillon" <bbrezillon@kernel.org>,
	"Jyri Sarha" <jyri.sarha@iki.fi>,
	virtualization@lists.linux-foundation.org,
	"Nicolas Ferre" <nicolas.ferre@microchip.com>,
	"Christian König" <christian.koenig@amd.com>,
	"Vincent Abriou" <vincent.abriou@st.com>,
	"Edmund Dea" <edmund.j.dea@intel.com>,
	freedreno@lists.freedesktop.org,
	"Daniel Vetter" <daniel@ffwll.ch>,
	"Kevin Hilman" <khilman@baylibre.com>,
	"Alex Deucher" <alexander.deucher@amd.com>,
	"Tian Tao" <tiantao6@hisilicon.com>,
	"Shawn Guo" <shawnguo@kernel.org>,
	"Brian Starkey" <brian.starkey@arm.com>,
	"Lucas Stach" <l.stach@pengutronix.de>
Subject: Re: [PATCH 05/10] drm: Use the state pointer directly in planes atomic_check
Date: Fri, 15 Jan 2021 22:42:14 +0200	[thread overview]
Message-ID: <YAH+JvNnlx73BA+Q@pendragon.ideasonboard.com> (raw)
In-Reply-To: <20210115125703.1315064-5-maxime@cerno.tech>

Hi Maxime,

Thank you for the patch.

On Fri, Jan 15, 2021 at 01:56:57PM +0100, Maxime Ripard wrote:
> Now that atomic_check takes the global atomic state as a parameter, we
> don't need to go through the pointer in the plane state.
> 
> This was done using the following coccinelle script:
> 
> @ plane_atomic_func @
> identifier helpers;
> identifier func;
> @@
> 
> static struct drm_plane_helper_funcs helpers = {
> 	...,
> 	.atomic_check = func,
> 	...,
> };
> 
> @@
> identifier plane_atomic_func.func;
> identifier plane, state;
> identifier plane_state;
> @@
> 
>   func(struct drm_plane *plane, struct drm_atomic_state *state) {
>   ...
> - struct drm_plane_state *plane_state = drm_atomic_get_new_plane_state(state, plane);
>   <... when != plane_state
> - plane_state->state
> + state
>   ...>
>  }
> 
> @@
> identifier plane_atomic_func.func;
> identifier plane, state;
> identifier plane_state;
> @@
> 
>   func(struct drm_plane *plane, struct drm_atomic_state *state) {
>   ...
>   struct drm_plane_state *plane_state = drm_atomic_get_new_plane_state(state, plane);
>   <...
> - plane_state->state
> + state
>   ...>
>  }
> 
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---

[snip]

>  drivers/gpu/drm/omapdrm/omap_plane.c              | 2 +-
>  drivers/gpu/drm/xlnx/zynqmp_disp.c                | 2 +-

For these, with the small issue below addressed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

[snip]

> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> index b0a3ba528718..d749acc78c85 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> @@ -1152,7 +1152,7 @@ zynqmp_disp_plane_atomic_check(struct drm_plane *plane,
>  	if (!new_plane_state->crtc)
>  		return 0;
>  
> -	crtc_state = drm_atomic_get_crtc_state(new_plane_state->state,
> +	crtc_state = drm_atomic_get_crtc_state(state,
>  					       new_plane_state->crtc);

This now holds on a single line.

>  	if (IS_ERR(crtc_state))
>  		return PTR_ERR(crtc_state);

[snip]

-- 
Regards,

Laurent Pinchart
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx