From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> To: Thomas Zimmermann <tzimmermann@suse.de> Cc: alexandre.belloni@bootlin.com, linux-aspeed@lists.ozlabs.org, narmstrong@baylibre.com, airlied@linux.ie, linus.walleij@linaro.org, liviu.dudau@arm.com, stefan@agner.ch, philippe.cornu@st.com, paul@crapouillou.net, benjamin.gaignard@linaro.org, mihail.atanassov@arm.com, sam@ravnborg.org, alexandre.torgue@st.com, marex@denx.de, festevam@gmail.com, abrodkin@synopsys.com, ludovic.desroches@microchip.com, xinliang.liu@linaro.org, kong.kongxinwei@hisilicon.com, tomi.valkeinen@ti.com, james.qian.wang@arm.com, joel@jms.id.au, linux-imx@nxp.com, p.zabel@pengutronix.de, puck.chen@hisilicon.com, s.hauer@pengutronix.de, alison.wang@nxp.com, maarten.lankhorst@linux.intel.com, mripard@kernel.org, john.stultz@linaro.org, jsarha@ti.com, wens@csie.org, vincent.abriou@st.com, kernel@pengutronix.de, linux-arm-kernel@lists.infradead.org, mcoquelin.stm32@gmail.com, noralf@tronnes.org, bbrezillon@kernel.org, andrew@aj.id.au, dri-devel@lists.freedesktop.org, yannick.fertre@st.com, kieran.bingham+renesas@ideasonboard.com, daniel@ffwll.ch, khilman@baylibre.com, zourongrong@gmail.com, shawnguo@kernel.org, brian.starkey@arm.com Subject: Re: [PATCH 15/21] drm/rcar-du: Use GEM CMA object functions Date: Fri, 22 May 2020 23:12:40 +0300 [thread overview] Message-ID: <20200522201240.GE5824@pendragon.ideasonboard.com> (raw) In-Reply-To: <20200522135246.10134-16-tzimmermann@suse.de> Hi Thomas, Thank you for the patch. On Fri, May 22, 2020 at 03:52:40PM +0200, Thomas Zimmermann wrote: > The rcar-du driver uses the default implementation for CMA functions; > except for the .dumb_create callback. The __DRM_GEM_CMA_DRIVER_OPS macro > now sets these defaults and .dumb_create in struct drm_driver. All > remaining operations are provided by CMA GEM object functions. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > drivers/gpu/drm/rcar-du/rcar_du_drv.c | 11 +---------- > 1 file changed, 1 insertion(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c > index 3e67cf70f0402..3728038cec1d1 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c > @@ -476,16 +476,7 @@ DEFINE_DRM_GEM_CMA_FOPS(rcar_du_fops); > > static struct drm_driver rcar_du_driver = { > .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC, > - .gem_free_object_unlocked = drm_gem_cma_free_object, > - .gem_vm_ops = &drm_gem_cma_vm_ops, > - .prime_handle_to_fd = drm_gem_prime_handle_to_fd, > - .prime_fd_to_handle = drm_gem_prime_fd_to_handle, > - .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table, > - .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, > - .gem_prime_vmap = drm_gem_cma_prime_vmap, > - .gem_prime_vunmap = drm_gem_cma_prime_vunmap, > - .gem_prime_mmap = drm_gem_cma_prime_mmap, > - .dumb_create = rcar_du_dumb_create, > + __DRM_GEM_CMA_DRIVER_OPS(rcar_du_dumb_create), Your __DRM_GEM_CMA_DRIVER_OPS is defined as #define __DRM_GEM_CMA_DRIVER_OPS(__dumb_create) \ .gem_create_object = drm_cma_gem_create_object_default_funcs, \ .dumb_create = (__dumb_create), \ .prime_handle_to_fd = drm_gem_prime_handle_to_fd, \ .prime_fd_to_handle = drm_gem_prime_fd_to_handle, \ .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table_vmap, \ .gem_prime_mmap = drm_gem_prime_mmap The patch thus introduces several changes: - drm_gem_cma_prime_import_sg_table_vmap() is used instead of drm_gem_cma_prime_import_sg_table() combined with .gem_prime_vmap() and .gem_prime_vunmap(). I believe that's fine, but splitting that change in a separate commit, or at the very least explaining it in details in the commit message, would make review easier. - .gem_create_object() is now set. That seems to be OK, but I'm not sure to grasp all the implications. This should also be explained in the commit message, and ideally split to a separate patch. - drm_gem_cma_prime_mmap() is replaced with drm_gem_prime_mmap(). Same comments :-) This patch hides way too many changes in what is documented as just innocent refactoring. It seems other drivers are affected too. > .fops = &rcar_du_fops, > .name = "rcar-du", > .desc = "Renesas R-Car Display Unit", -- Regards, Laurent Pinchart _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> To: Thomas Zimmermann <tzimmermann@suse.de> Cc: alexandre.belloni@bootlin.com, linux-aspeed@lists.ozlabs.org, narmstrong@baylibre.com, airlied@linux.ie, liviu.dudau@arm.com, philippe.cornu@st.com, paul@crapouillou.net, mihail.atanassov@arm.com, sam@ravnborg.org, alexandre.torgue@st.com, marex@denx.de, abrodkin@synopsys.com, ludovic.desroches@microchip.com, xinliang.liu@linaro.org, kong.kongxinwei@hisilicon.com, tomi.valkeinen@ti.com, james.qian.wang@arm.com, joel@jms.id.au, linux-imx@nxp.com, puck.chen@hisilicon.com, s.hauer@pengutronix.de, alison.wang@nxp.com, jsarha@ti.com, wens@csie.org, vincent.abriou@st.com, kernel@pengutronix.de, linux-arm-kernel@lists.infradead.org, mcoquelin.stm32@gmail.com, bbrezillon@kernel.org, andrew@aj.id.au, dri-devel@lists.freedesktop.org, nicolas.ferre@microchip.com, yannick.fertre@st.com, kieran.bingham+renesas@ideasonboard.com, khilman@baylibre.com, zourongrong@gmail.com, shawnguo@kernel.org Subject: Re: [PATCH 15/21] drm/rcar-du: Use GEM CMA object functions Date: Fri, 22 May 2020 23:12:40 +0300 [thread overview] Message-ID: <20200522201240.GE5824@pendragon.ideasonboard.com> (raw) In-Reply-To: <20200522135246.10134-16-tzimmermann@suse.de> Hi Thomas, Thank you for the patch. On Fri, May 22, 2020 at 03:52:40PM +0200, Thomas Zimmermann wrote: > The rcar-du driver uses the default implementation for CMA functions; > except for the .dumb_create callback. The __DRM_GEM_CMA_DRIVER_OPS macro > now sets these defaults and .dumb_create in struct drm_driver. All > remaining operations are provided by CMA GEM object functions. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > drivers/gpu/drm/rcar-du/rcar_du_drv.c | 11 +---------- > 1 file changed, 1 insertion(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c > index 3e67cf70f0402..3728038cec1d1 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c > @@ -476,16 +476,7 @@ DEFINE_DRM_GEM_CMA_FOPS(rcar_du_fops); > > static struct drm_driver rcar_du_driver = { > .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC, > - .gem_free_object_unlocked = drm_gem_cma_free_object, > - .gem_vm_ops = &drm_gem_cma_vm_ops, > - .prime_handle_to_fd = drm_gem_prime_handle_to_fd, > - .prime_fd_to_handle = drm_gem_prime_fd_to_handle, > - .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table, > - .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table, > - .gem_prime_vmap = drm_gem_cma_prime_vmap, > - .gem_prime_vunmap = drm_gem_cma_prime_vunmap, > - .gem_prime_mmap = drm_gem_cma_prime_mmap, > - .dumb_create = rcar_du_dumb_create, > + __DRM_GEM_CMA_DRIVER_OPS(rcar_du_dumb_create), Your __DRM_GEM_CMA_DRIVER_OPS is defined as #define __DRM_GEM_CMA_DRIVER_OPS(__dumb_create) \ .gem_create_object = drm_cma_gem_create_object_default_funcs, \ .dumb_create = (__dumb_create), \ .prime_handle_to_fd = drm_gem_prime_handle_to_fd, \ .prime_fd_to_handle = drm_gem_prime_fd_to_handle, \ .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table_vmap, \ .gem_prime_mmap = drm_gem_prime_mmap The patch thus introduces several changes: - drm_gem_cma_prime_import_sg_table_vmap() is used instead of drm_gem_cma_prime_import_sg_table() combined with .gem_prime_vmap() and .gem_prime_vunmap(). I believe that's fine, but splitting that change in a separate commit, or at the very least explaining it in details in the commit message, would make review easier. - .gem_create_object() is now set. That seems to be OK, but I'm not sure to grasp all the implications. This should also be explained in the commit message, and ideally split to a separate patch. - drm_gem_cma_prime_mmap() is replaced with drm_gem_prime_mmap(). Same comments :-) This patch hides way too many changes in what is documented as just innocent refactoring. It seems other drivers are affected too. > .fops = &rcar_du_fops, > .name = "rcar-du", > .desc = "Renesas R-Car Display Unit", -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2020-05-22 20:12 UTC|newest] Thread overview: 92+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-05-22 13:52 [PATCH 00/21] drm: Convert most CMA-based drivers to GEM object functions Thomas Zimmermann 2020-05-22 13:52 ` [PATCH 01/21] drm/cma-helper: Rework DRM_GEM_CMA_VMAP_DRIVER_OPS macro Thomas Zimmermann 2020-05-22 17:48 ` Sam Ravnborg 2020-05-22 17:48 ` Sam Ravnborg 2020-05-22 18:15 ` Emil Velikov 2020-05-22 18:15 ` Emil Velikov 2020-05-22 18:45 ` Sam Ravnborg 2020-05-22 18:45 ` Sam Ravnborg 2020-05-22 19:48 ` Laurent Pinchart 2020-05-22 19:48 ` Laurent Pinchart 2020-05-25 12:03 ` Thomas Zimmermann 2020-05-25 12:03 ` Thomas Zimmermann 2020-05-22 13:52 ` [PATCH 02/21] drm/arc: Use GEM CMA object functions Thomas Zimmermann 2020-05-22 13:52 ` [PATCH 03/21] drm/arm: " Thomas Zimmermann 2020-05-26 23:30 ` Liviu Dudau 2020-05-26 23:30 ` Liviu Dudau 2020-05-22 13:52 ` [PATCH 04/21] drm/aspeed: Set driver CMA functions with DRM_GEM_CMA_DRIVER_OPS Thomas Zimmermann 2020-10-09 7:54 ` Joel Stanley 2020-10-09 7:54 ` Joel Stanley 2020-10-09 8:01 ` Thomas Zimmermann 2020-10-09 8:01 ` Thomas Zimmermann 2020-10-09 8:06 ` Joel Stanley 2020-10-09 8:06 ` Joel Stanley 2020-10-09 8:26 ` Thomas Zimmermann 2020-10-09 8:26 ` Thomas Zimmermann 2020-10-09 10:45 ` Joel Stanley 2020-10-09 10:45 ` Joel Stanley 2020-05-22 13:52 ` [PATCH 05/21] drm/atmel-hlcdc: Use GEM CMA object functions Thomas Zimmermann 2020-05-22 18:08 ` Sam Ravnborg 2020-05-22 18:08 ` Sam Ravnborg 2020-05-25 12:10 ` Thomas Zimmermann 2020-05-25 12:10 ` Thomas Zimmermann 2020-05-22 19:25 ` Sam Ravnborg 2020-05-22 19:25 ` Sam Ravnborg 2020-05-25 12:37 ` Thomas Zimmermann 2020-05-25 12:37 ` Thomas Zimmermann 2020-05-22 13:52 ` [PATCH 06/21] drm/fsl-dcu: " Thomas Zimmermann 2020-05-22 13:52 ` [PATCH 07/21] drm/hisilicon/kirin: " Thomas Zimmermann 2020-05-22 18:11 ` Emil Velikov 2020-05-25 12:41 ` Thomas Zimmermann 2020-05-25 12:41 ` Thomas Zimmermann 2020-05-28 14:34 ` Emil Velikov 2020-05-28 14:34 ` Emil Velikov 2020-05-22 13:52 ` [PATCH 08/21] drm/imx: " Thomas Zimmermann 2020-05-22 13:52 ` [PATCH 09/21] drm/ingenic: " Thomas Zimmermann 2020-05-27 0:28 ` Paul Cercueil 2020-05-27 0:28 ` Paul Cercueil 2020-05-22 13:52 ` [PATCH 10/21] drm/komeda: " Thomas Zimmermann 2020-05-26 23:33 ` Liviu Dudau 2020-05-26 23:33 ` Liviu Dudau 2020-05-22 13:52 ` [PATCH 11/21] drm/malidp: " Thomas Zimmermann 2020-05-26 23:33 ` Liviu Dudau 2020-05-26 23:33 ` Liviu Dudau 2020-05-22 13:52 ` [PATCH 12/21] drm/mcde: " Thomas Zimmermann 2020-05-25 11:36 ` Linus Walleij 2020-05-25 11:36 ` Linus Walleij 2020-05-25 12:51 ` Thomas Zimmermann 2020-05-25 12:51 ` Thomas Zimmermann 2020-05-25 13:08 ` Linus Walleij 2020-05-25 13:08 ` Linus Walleij 2020-05-25 13:27 ` Thomas Zimmermann 2020-05-25 13:27 ` Thomas Zimmermann 2020-05-22 13:52 ` [PATCH 13/21] drm/meson: " Thomas Zimmermann 2020-05-22 13:52 ` [PATCH 14/21] drm/mxsfb: " Thomas Zimmermann 2020-05-22 13:52 ` [PATCH 15/21] drm/rcar-du: " Thomas Zimmermann 2020-05-22 20:12 ` Laurent Pinchart [this message] 2020-05-22 20:12 ` Laurent Pinchart 2020-05-25 12:49 ` Thomas Zimmermann 2020-05-25 12:49 ` Thomas Zimmermann 2020-05-25 15:38 ` Kieran Bingham 2020-05-25 15:38 ` Kieran Bingham 2020-06-03 7:48 ` Thomas Zimmermann 2020-06-03 7:48 ` Thomas Zimmermann 2020-05-26 1:50 ` Laurent Pinchart 2020-05-26 1:50 ` Laurent Pinchart 2020-05-22 13:52 ` [PATCH 16/21] drm/shmobile: " Thomas Zimmermann 2020-05-22 13:52 ` [PATCH 17/21] drm/stm: " Thomas Zimmermann 2020-05-26 13:22 ` Philippe CORNU 2020-05-26 13:22 ` Philippe CORNU 2020-05-22 13:52 ` [PATCH 18/21] drm/sti: " Thomas Zimmermann 2020-05-22 13:52 ` [PATCH 19/21] drm/tilcdc: " Thomas Zimmermann 2020-05-22 13:52 ` [PATCH 20/21] drm/tv200: " Thomas Zimmermann 2020-05-25 11:36 ` Linus Walleij 2020-05-25 11:36 ` Linus Walleij 2020-05-22 13:52 ` [PATCH 21/21] drm/zte: " Thomas Zimmermann 2020-05-22 19:03 ` Sam Ravnborg 2020-05-22 19:03 ` Sam Ravnborg 2020-05-24 14:50 ` kbuild test robot 2020-05-24 14:50 ` kbuild test robot 2020-05-24 20:11 ` kbuild test robot 2020-05-24 20:11 ` kbuild test robot 2020-05-22 18:19 ` [PATCH 00/21] drm: Convert most CMA-based drivers to GEM " Emil Velikov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200522201240.GE5824@pendragon.ideasonboard.com \ --to=laurent.pinchart@ideasonboard.com \ --cc=abrodkin@synopsys.com \ --cc=airlied@linux.ie \ --cc=alexandre.belloni@bootlin.com \ --cc=alexandre.torgue@st.com \ --cc=alison.wang@nxp.com \ --cc=andrew@aj.id.au \ --cc=bbrezillon@kernel.org \ --cc=benjamin.gaignard@linaro.org \ --cc=brian.starkey@arm.com \ --cc=daniel@ffwll.ch \ --cc=dri-devel@lists.freedesktop.org \ --cc=festevam@gmail.com \ --cc=james.qian.wang@arm.com \ --cc=joel@jms.id.au \ --cc=john.stultz@linaro.org \ --cc=jsarha@ti.com \ --cc=kernel@pengutronix.de \ --cc=khilman@baylibre.com \ --cc=kieran.bingham+renesas@ideasonboard.com \ --cc=kong.kongxinwei@hisilicon.com \ --cc=linus.walleij@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-aspeed@lists.ozlabs.org \ --cc=linux-imx@nxp.com \ --cc=liviu.dudau@arm.com \ --cc=ludovic.desroches@microchip.com \ --cc=maarten.lankhorst@linux.intel.com \ --cc=marex@denx.de \ --cc=mcoquelin.stm32@gmail.com \ --cc=mihail.atanassov@arm.com \ --cc=mripard@kernel.org \ --cc=narmstrong@baylibre.com \ --cc=noralf@tronnes.org \ --cc=p.zabel@pengutronix.de \ --cc=paul@crapouillou.net \ --cc=philippe.cornu@st.com \ --cc=puck.chen@hisilicon.com \ --cc=s.hauer@pengutronix.de \ --cc=sam@ravnborg.org \ --cc=shawnguo@kernel.org \ --cc=stefan@agner.ch \ --cc=tomi.valkeinen@ti.com \ --cc=tzimmermann@suse.de \ --cc=vincent.abriou@st.com \ --cc=wens@csie.org \ --cc=xinliang.liu@linaro.org \ --cc=yannick.fertre@st.com \ --cc=zourongrong@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.