From: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> To: Thomas Zimmermann <tzimmermann@suse.de>, Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: alexandre.belloni@bootlin.com, linux-aspeed@lists.ozlabs.org, narmstrong@baylibre.com, airlied@linux.ie, liviu.dudau@arm.com, dri-devel@lists.freedesktop.org, paul@crapouillou.net, mihail.atanassov@arm.com, sam@ravnborg.org, marex@denx.de, khilman@baylibre.com, abrodkin@synopsys.com, kong.kongxinwei@hisilicon.com, xinliang.liu@linaro.org, ludovic.desroches@microchip.com, tomi.valkeinen@ti.com, james.qian.wang@arm.com, joel@jms.id.au, linux-imx@nxp.com, alexandre.torgue@st.com, puck.chen@hisilicon.com, s.hauer@pengutronix.de, alison.wang@nxp.com, jsarha@ti.com, wens@csie.org, vincent.abriou@st.com, linux-arm-kernel@lists.infradead.org, mcoquelin.stm32@gmail.com, bbrezillon@kernel.org, andrew@aj.id.au, philippe.cornu@st.com, yannick.fertre@st.com, kernel@pengutronix.de, zourongrong@gmail.com, shawnguo@kernel.org Subject: Re: [PATCH 15/21] drm/rcar-du: Use GEM CMA object functions Date: Mon, 25 May 2020 16:38:41 +0100 [thread overview] Message-ID: <50d76988-f627-037d-a8bc-d18f6662c981@ideasonboard.com> (raw) In-Reply-To: <816a8a0e-bb98-ea6c-5016-94b18e045fb5@suse.de> On 25/05/2020 13:49, Thomas Zimmermann wrote: > Hi > > Am 22.05.20 um 22:12 schrieb Laurent Pinchart: >> 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. > > That's relevant during object creation and sets the object functions. > See one of my other replies for how this can go away after all CMA > drivers have been updated to GEM object functions. > > >> >> - drm_gem_cma_prime_mmap() is replaced with drm_gem_prime_mmap(). Same >> comments :-) > > I relied on the aspeed driver to be correct. After Sam's comment on > that, I read the code once again several times. The original > implementation clears VM_PFNMAP. And I cannot find that code any longer. > Going back to the original function might be better. > > >> >> This patch hides way too many changes in what is documented as just >> innocent refactoring. It seems other drivers are affected too. > > Could you test the patchset? I don't have the HW. Digging out the branch you provided elsewhere in this thread: >>> Could you boot-test with the patchset applied? >> >> Yes, if you have a git branch I can just build and boot I can >> do it quickly! > > Fantastic! It's the cma-objfuncs branch of > > https://gitlab.freedesktop.org/tzimmermann/linux.git I have successfully run our display tests with your patches here on an R-Car H3 Salvator-XS(-es2). Tested-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > Best regards > Thomas > >> >>> .fops = &rcar_du_fops, >>> .name = "rcar-du", >>> .desc = "Renesas R-Car Display Unit", >> > _______________________________________________ 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: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> To: Thomas Zimmermann <tzimmermann@suse.de>, Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: alexandre.belloni@bootlin.com, linux-aspeed@lists.ozlabs.org, narmstrong@baylibre.com, airlied@linux.ie, liviu.dudau@arm.com, dri-devel@lists.freedesktop.org, nicolas.ferre@microchip.com, paul@crapouillou.net, mihail.atanassov@arm.com, sam@ravnborg.org, marex@denx.de, khilman@baylibre.com, abrodkin@synopsys.com, kong.kongxinwei@hisilicon.com, xinliang.liu@linaro.org, ludovic.desroches@microchip.com, tomi.valkeinen@ti.com, james.qian.wang@arm.com, joel@jms.id.au, linux-imx@nxp.com, alexandre.torgue@st.com, puck.chen@hisilicon.com, s.hauer@pengutronix.de, alison.wang@nxp.com, jsarha@ti.com, wens@csie.org, vincent.abriou@st.com, linux-arm-kernel@lists.infradead.org, mcoquelin.stm32@gmail.com, bbrezillon@kernel.org, andrew@aj.id.au, philippe.cornu@st.com, yannick.fertre@st.com, kernel@pengutronix.de, zourongrong@gmail.com, shawnguo@kernel.org Subject: Re: [PATCH 15/21] drm/rcar-du: Use GEM CMA object functions Date: Mon, 25 May 2020 16:38:41 +0100 [thread overview] Message-ID: <50d76988-f627-037d-a8bc-d18f6662c981@ideasonboard.com> (raw) In-Reply-To: <816a8a0e-bb98-ea6c-5016-94b18e045fb5@suse.de> On 25/05/2020 13:49, Thomas Zimmermann wrote: > Hi > > Am 22.05.20 um 22:12 schrieb Laurent Pinchart: >> 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. > > That's relevant during object creation and sets the object functions. > See one of my other replies for how this can go away after all CMA > drivers have been updated to GEM object functions. > > >> >> - drm_gem_cma_prime_mmap() is replaced with drm_gem_prime_mmap(). Same >> comments :-) > > I relied on the aspeed driver to be correct. After Sam's comment on > that, I read the code once again several times. The original > implementation clears VM_PFNMAP. And I cannot find that code any longer. > Going back to the original function might be better. > > >> >> This patch hides way too many changes in what is documented as just >> innocent refactoring. It seems other drivers are affected too. > > Could you test the patchset? I don't have the HW. Digging out the branch you provided elsewhere in this thread: >>> Could you boot-test with the patchset applied? >> >> Yes, if you have a git branch I can just build and boot I can >> do it quickly! > > Fantastic! It's the cma-objfuncs branch of > > https://gitlab.freedesktop.org/tzimmermann/linux.git I have successfully run our display tests with your patches here on an R-Car H3 Salvator-XS(-es2). Tested-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > Best regards > Thomas > >> >>> .fops = &rcar_du_fops, >>> .name = "rcar-du", >>> .desc = "Renesas R-Car Display Unit", >> > _______________________________________________ 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-25 15:47 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 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 [this message] 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=50d76988-f627-037d-a8bc-d18f6662c981@ideasonboard.com \ --to=kieran.bingham+renesas@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=dri-devel@lists.freedesktop.org \ --cc=james.qian.wang@arm.com \ --cc=joel@jms.id.au \ --cc=jsarha@ti.com \ --cc=kernel@pengutronix.de \ --cc=khilman@baylibre.com \ --cc=kong.kongxinwei@hisilicon.com \ --cc=laurent.pinchart@ideasonboard.com \ --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=marex@denx.de \ --cc=mcoquelin.stm32@gmail.com \ --cc=mihail.atanassov@arm.com \ --cc=narmstrong@baylibre.com \ --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=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.