drm: CMA fbdev stride adjustment

PATCH In review Rob Herring 2 participants 4 messages Aug 12, 2015 – Aug 13, 2015

Subsystems (8)

DRM drivers DRM misc Arm i.MX and Layerscape Arm/Microchip (AT91) SoC support +4 more subsystems

lore

Share & tools

lore

Rob Herring

author

2015-08-12 20:52 UTC #0 lore raw

cover drm: CMA fbdev stride adjustment

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(-)

Signature · 6 lines

2.1.4

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Rob Herring

author

2015-08-12 20:52 UTC #1 lore raw

PATCH 1/2 drm/cma: allow custom fb helper functions

Allow drivers to provide custom drm_fb_helper_funcs overriding the default functions.

Signed-off-by Rob Herring <[email protected]>

---
 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(-)

drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c +1 -1

index 6fad1f9..f8047ab 100644
@@ -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))

drivers/gpu/drm/drm_fb_cma_helper.c +8 -2

index 5c1aca4..f7751a2 100644
@@ -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) {

drivers/gpu/drm/imx/imx-drm-core.c +1 -1

index 74f505b..4da348b 100644
@@ -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);

drivers/gpu/drm/rcar-du/rcar_du_kms.c +2 -1

index 56518eb..7546408 100644
@@ -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);

drivers/gpu/drm/sti/sti_drm_drv.c +1 -1

index 59d558b..64d1fb1 100644
@@ -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

drivers/gpu/drm/tilcdc/tilcdc_drv.c +1 -1

index 0f283a3..cab0df6 100644
@@ -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)) {

include/drm/drm_fb_cma_helper.h +7 -0

index c54cf3d..ef691ab 100644
@@ -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);

Signature · 6 lines

2.1.4

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Rob Herring

author

2015-08-12 20:52 UTC #2 lore raw

PATCH 2/2 drm/cma: allow adjusting the pitch for CMA fbdev

In reply to Rob Herring

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 <[email protected]>

---
 drivers/gpu/drm/drm_fb_cma_helper.c | 3 ++-
 include/drm/drm_fb_helper.h         | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

drivers/gpu/drm/drm_fb_cma_helper.c +2 -1

index f7751a2..b393e5c 100644
@@ -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);

include/drm/drm_fb_helper.h +1 -0

index 0dfd94def..8072c4f 100644
@@ -70,6 +70,7 @@ struct drm_fb_helper_surface_size {
 	u32 surface_height;
 	u32 surface_bpp;
 	u32 surface_depth;
+	u32 surface_pitch;
 };
 
 /**

Signature · 6 lines

2.1.4

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Daniel Vetter

2015-08-13 06:58 UTC #3 lore raw

In reply to Rob Herring

Quoted 7 lines · Rob Herring

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.

Quoted 7 lines

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

Quoted 24 lines

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(-)

Signature · 6 lines

2.1.4

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Signature · 7 lines

Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel