I'm working on a DRM driver for PXA1928. Other than a stride alignment requirement of 16 bytes, I have no other reason not to use fbdev_cma. While I can adjust the stride for drm_gem_cma_dumb_create, I cannot do the same for drm_fbdev_cma_create without duplicating a bunch of code. This series allows fbdev_cma users to override the fb_probe function, so the stride can be adjusted.
It appears to me that rcar-du has a bug that it doesn't handle alignment requirements for this case as well. Probably just getting lucky with tested resolutions/bpp.
Also, AFAICT the Rockchip driver has no real reason to use a custom GEM allocator instead of the CMA one. It sets the DMA_ATTR_NO_KERNEL_MAPPING DMA attr, but that could easily be supported by the CMA allocator.
Rob
Rob Herring (2): drm/cma: allow custom fb helper functions drm/cma: allow adjusting the pitch for CMA fbdev
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 2 +- drivers/gpu/drm/drm_fb_cma_helper.c | 13 ++++++++++--- drivers/gpu/drm/imx/imx-drm-core.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_kms.c | 3 ++- drivers/gpu/drm/sti/sti_drm_drv.c | 2 +- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 +- include/drm/drm_fb_cma_helper.h | 7 +++++++ include/drm/drm_fb_helper.h | 1 + 8 files changed, 24 insertions(+), 8 deletions(-)
Allow drivers to provide custom drm_fb_helper_funcs overriding the default functions.
Signed-off-by: Rob Herring robh@kernel.org --- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 2 +- drivers/gpu/drm/drm_fb_cma_helper.c | 10 ++++++++-- drivers/gpu/drm/imx/imx-drm-core.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_kms.c | 3 ++- drivers/gpu/drm/sti/sti_drm_drv.c | 2 +- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 +- include/drm/drm_fb_cma_helper.h | 7 +++++++ 7 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c index 6fad1f9..f8047ab 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c @@ -211,7 +211,7 @@ static void atmel_hlcdc_fb_output_poll_changed(struct drm_device *dev) if (dc->fbdev) { drm_fbdev_cma_hotplug_event(dc->fbdev); } else { - dc->fbdev = drm_fbdev_cma_init(dev, 24, + dc->fbdev = drm_fbdev_cma_init(dev, NULL, 24, dev->mode_config.num_crtc, dev->mode_config.num_connector); if (IS_ERR(dc->fbdev)) diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c index 5c1aca4..f7751a2 100644 --- a/drivers/gpu/drm/drm_fb_cma_helper.c +++ b/drivers/gpu/drm/drm_fb_cma_helper.c @@ -244,7 +244,7 @@ static struct fb_ops drm_fbdev_cma_ops = { .fb_setcmap = drm_fb_helper_setcmap, };
-static int drm_fbdev_cma_create(struct drm_fb_helper *helper, +int drm_fbdev_cma_create(struct drm_fb_helper *helper, struct drm_fb_helper_surface_size *sizes) { struct drm_fbdev_cma *fbdev_cma = to_fbdev_cma(helper); @@ -326,6 +326,7 @@ err_drm_gem_cma_free_object: drm_gem_cma_free_object(&obj->base); return ret; } +EXPORT_SYMBOL_GPL(drm_fbdev_cma_create);
static const struct drm_fb_helper_funcs drm_fb_cma_helper_funcs = { .fb_probe = drm_fbdev_cma_create, @@ -334,6 +335,7 @@ static const struct drm_fb_helper_funcs drm_fb_cma_helper_funcs = { /** * drm_fbdev_cma_init() - Allocate and initializes a drm_fbdev_cma struct * @dev: DRM device + * @helper_funcs: Optional driver specific drm_fb_helper_funcs * @preferred_bpp: Preferred bits per pixel for the device * @num_crtc: Number of CRTCs * @max_conn_count: Maximum number of connectors @@ -341,6 +343,7 @@ static const struct drm_fb_helper_funcs drm_fb_cma_helper_funcs = { * Returns a newly allocated drm_fbdev_cma struct or a ERR_PTR. */ struct drm_fbdev_cma *drm_fbdev_cma_init(struct drm_device *dev, + const struct drm_fb_helper_funcs *helper_funcs, unsigned int preferred_bpp, unsigned int num_crtc, unsigned int max_conn_count) { @@ -356,7 +359,10 @@ struct drm_fbdev_cma *drm_fbdev_cma_init(struct drm_device *dev,
helper = &fbdev_cma->fb_helper;
- drm_fb_helper_prepare(dev, helper, &drm_fb_cma_helper_funcs); + if (!helper_funcs) + helper_funcs = &drm_fb_cma_helper_funcs; + + drm_fb_helper_prepare(dev, helper, helper_funcs);
ret = drm_fb_helper_init(dev, helper, num_crtc, max_conn_count); if (ret < 0) { diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c index 74f505b..4da348b 100644 --- a/drivers/gpu/drm/imx/imx-drm-core.c +++ b/drivers/gpu/drm/imx/imx-drm-core.c @@ -313,7 +313,7 @@ static int imx_drm_driver_load(struct drm_device *drm, unsigned long flags) dev_warn(drm->dev, "Invalid legacyfb_depth. Defaulting to 16bpp\n"); legacyfb_depth = 16; } - imxdrm->fbhelper = drm_fbdev_cma_init(drm, legacyfb_depth, + imxdrm->fbhelper = drm_fbdev_cma_init(drm, NULL, legacyfb_depth, drm->mode_config.num_crtc, MAX_CRTC); if (IS_ERR(imxdrm->fbhelper)) { ret = PTR_ERR(imxdrm->fbhelper); diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index 56518eb..7546408 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c @@ -828,7 +828,8 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu) drm_kms_helper_poll_init(dev);
if (dev->mode_config.num_connector) { - fbdev = drm_fbdev_cma_init(dev, 32, dev->mode_config.num_crtc, + fbdev = drm_fbdev_cma_init(dev, NULL, 32, + dev->mode_config.num_crtc, dev->mode_config.num_connector); if (IS_ERR(fbdev)) return PTR_ERR(fbdev); diff --git a/drivers/gpu/drm/sti/sti_drm_drv.c b/drivers/gpu/drm/sti/sti_drm_drv.c index 59d558b..64d1fb1 100644 --- a/drivers/gpu/drm/sti/sti_drm_drv.c +++ b/drivers/gpu/drm/sti/sti_drm_drv.c @@ -161,7 +161,7 @@ static int sti_drm_load(struct drm_device *dev, unsigned long flags) drm_mode_config_reset(dev);
#ifdef CONFIG_DRM_STI_FBDEV - drm_fbdev_cma_init(dev, 32, + drm_fbdev_cma_init(dev, NULL, 32, dev->mode_config.num_crtc, dev->mode_config.num_connector); #endif diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c index 0f283a3..cab0df6 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c @@ -294,7 +294,7 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags) break; }
- priv->fbdev = drm_fbdev_cma_init(dev, bpp, + priv->fbdev = drm_fbdev_cma_init(dev, NULL, bpp, dev->mode_config.num_crtc, dev->mode_config.num_connector); if (IS_ERR(priv->fbdev)) { diff --git a/include/drm/drm_fb_cma_helper.h b/include/drm/drm_fb_cma_helper.h index c54cf3d..ef691ab 100644 --- a/include/drm/drm_fb_cma_helper.h +++ b/include/drm/drm_fb_cma_helper.h @@ -8,12 +8,19 @@ struct drm_framebuffer; struct drm_device; struct drm_file; struct drm_mode_fb_cmd2; +struct drm_fb_helper_funcs; +struct drm_fb_helper; +struct drm_fb_helper_surface_size;
struct drm_fbdev_cma *drm_fbdev_cma_init(struct drm_device *dev, + const struct drm_fb_helper_funcs *helper_funcs, unsigned int preferred_bpp, unsigned int num_crtc, unsigned int max_conn_count); void drm_fbdev_cma_fini(struct drm_fbdev_cma *fbdev_cma);
+int drm_fbdev_cma_create(struct drm_fb_helper *helper, + struct drm_fb_helper_surface_size *sizes); + void drm_fbdev_cma_restore_mode(struct drm_fbdev_cma *fbdev_cma); void drm_fbdev_cma_hotplug_event(struct drm_fbdev_cma *fbdev_cma);
Some hardware has pitch alignment requirements, but there is no way to set the pitch for CMA fbdev other than completely replacing drm_fbdev_cma_create. Add the pitch and allow drivers to adjust it before calling drm_fbdev_cma_create.
Signed-off-by: Rob Herring robh@kernel.org --- drivers/gpu/drm/drm_fb_cma_helper.c | 3 ++- include/drm/drm_fb_helper.h | 1 + 2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c index f7751a2..b393e5c 100644 --- a/drivers/gpu/drm/drm_fb_cma_helper.c +++ b/drivers/gpu/drm/drm_fb_cma_helper.c @@ -266,7 +266,8 @@ int drm_fbdev_cma_create(struct drm_fb_helper *helper,
mode_cmd.width = sizes->surface_width; mode_cmd.height = sizes->surface_height; - mode_cmd.pitches[0] = sizes->surface_width * bytes_per_pixel; + mode_cmd.pitches[0] = sizes->surface_pitch ? + sizes->surface_pitch : sizes->surface_width * bytes_per_pixel; mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp, sizes->surface_depth);
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h index 0dfd94def..8072c4f 100644 --- a/include/drm/drm_fb_helper.h +++ b/include/drm/drm_fb_helper.h @@ -70,6 +70,7 @@ struct drm_fb_helper_surface_size { u32 surface_height; u32 surface_bpp; u32 surface_depth; + u32 surface_pitch; };
/**
On Wed, Aug 12, 2015 at 03:52:12PM -0500, Rob Herring wrote:
I'm working on a DRM driver for PXA1928. Other than a stride alignment requirement of 16 bytes, I have no other reason not to use fbdev_cma. While I can adjust the stride for drm_gem_cma_dumb_create, I cannot do the same for drm_fbdev_cma_create without duplicating a bunch of code. This series allows fbdev_cma users to override the fb_probe function, so the stride can be adjusted.
It appears to me that rcar-du has a bug that it doesn't handle alignment requirements for this case as well. Probably just getting lucky with tested resolutions/bpp.
Also, AFAICT the Rockchip driver has no real reason to use a custom GEM allocator instead of the CMA one. It sets the DMA_ATTR_NO_KERNEL_MAPPING DMA attr, but that could easily be supported by the CMA allocator.
I think it'd be easier to review this with the driver at hand. That's also generally the requirement for merging new code - it needs an in-kernel user. -Daniel
Rob
Rob Herring (2): drm/cma: allow custom fb helper functions drm/cma: allow adjusting the pitch for CMA fbdev
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 2 +- drivers/gpu/drm/drm_fb_cma_helper.c | 13 ++++++++++--- drivers/gpu/drm/imx/imx-drm-core.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_kms.c | 3 ++- drivers/gpu/drm/sti/sti_drm_drv.c | 2 +- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 +- include/drm/drm_fb_cma_helper.h | 7 +++++++ include/drm/drm_fb_helper.h | 1 + 8 files changed, 24 insertions(+), 8 deletions(-)
-- 2.1.4
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Rob Herring