[PATCH 00/20] drm/cma-helper drivers: Use drm_fbdev_generic_setup()

[PATCH 00/20] drm/cma-helper drivers: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

This patchset moves the drivers using the CMA helper fully over to the
generic fbdev emulation. The unused fbdev code is removed from the CMA
helper.

Noralf.

Noralf Trønnes (20):
  drm/fb-helper: Improve error reporting in setup
  drm/arc: Use drm_fbdev_generic_setup()
  drm/fsl-dcu: Use drm_fbdev_generic_setup()
  drm/hisilicon/kirin: Use drm_fbdev_generic_setup()
  drm/meson: Use drm_fbdev_generic_setup()
  drm/mxsfb: Use drm_fbdev_generic_setup()
  drm/rcar-du: Use drm_fbdev_generic_setup()
  drm/arm/hdlcd: Use drm_fbdev_generic_setup()
  drm/arm/mali: Use drm_fbdev_generic_setup()
  drm/atmel-hlcdc: Use drm_fbdev_generic_setup()
  drm/imx: Use drm_fbdev_generic_setup()
  drm/pl111: Use drm_fbdev_generic_setup()
  drm/sti: Use drm_fbdev_generic_setup()
  drm/stm: Use drm_fbdev_generic_setup()
  drm/sun4i: Use drm_fbdev_generic_setup()
  drm/tilcdc: Use drm_fbdev_generic_setup()
  drm/tve200: Use drm_fbdev_generic_setup()
  drm/vc4: Use drm_fbdev_generic_setup()
  drm/zte: Use drm_fbdev_generic_setup()
  drm/cma-helper: Remove unused fbdev code

 drivers/gpu/drm/Kconfig                         |   4 -
 drivers/gpu/drm/arc/arcpgu.h                    |   4 -
 drivers/gpu/drm/arc/arcpgu_drv.c                |  33 +-----
 drivers/gpu/drm/arm/hdlcd_drv.c                 |  11 +-
 drivers/gpu/drm/arm/malidp_drv.c                |  11 +-
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c    |   7 +-
 drivers/gpu/drm/drm_fb_cma_helper.c             | 151 ------------------------
 drivers/gpu/drm/drm_fb_helper.c                 |   8 +-
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c       |  25 +---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h       |   1 -
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c |  38 +-----
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h |   4 -
 drivers/gpu/drm/imx/imx-drm-core.c              |  19 +--
 drivers/gpu/drm/meson/meson_drv.c               |  19 +--
 drivers/gpu/drm/meson/meson_drv.h               |   1 -
 drivers/gpu/drm/mxsfb/mxsfb_drv.c               |  26 +---
 drivers/gpu/drm/mxsfb/mxsfb_drv.h               |   1 -
 drivers/gpu/drm/pl111/pl111_drv.c               |   6 +-
 drivers/gpu/drm/rcar-du/rcar_du_drv.c           |  34 +-----
 drivers/gpu/drm/rcar-du/rcar_du_drv.h           |   3 -
 drivers/gpu/drm/rcar-du/rcar_du_kms.c           |  21 ----
 drivers/gpu/drm/sti/sti_drv.c                   |   8 +-
 drivers/gpu/drm/stm/drv.c                       |  11 +-
 drivers/gpu/drm/sun4i/sun4i_drv.c               |  12 +-
 drivers/gpu/drm/sun4i/sun4i_framebuffer.c       |  12 +-
 drivers/gpu/drm/sun4i/sun4i_framebuffer.h       |   3 +-
 drivers/gpu/drm/tilcdc/tilcdc_drv.c             |  11 +-
 drivers/gpu/drm/tve200/tve200_drv.c             |  14 +--
 drivers/gpu/drm/vc4/vc4_drv.c                   |   5 +-
 drivers/gpu/drm/vc4/vc4_kms.c                   |   6 -
 drivers/gpu/drm/zte/zx_drm_drv.c                |  15 +--
 include/drm/drm_fb_cma_helper.h                 |  24 ----
 32 files changed, 56 insertions(+), 492 deletions(-)

-- 
2.15.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 01/20] drm/fb-helper: Improve error reporting in setup

[Noralf Trønnes]

Improve error reporting in drm_fb_helper_fbdev_setup() by printing the
error code. This is useful for drivers that choose to not fall over just
because fbdev doesen't work, but still wants clues to why it failed.
This way they don't have to provide an error message themselves.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_fb_helper.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 4b0dd20bccb8..2f865a046f6d 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2842,7 +2842,7 @@ int drm_fb_helper_fbdev_setup(struct drm_device *dev,
 	if (!max_conn_count)
 		max_conn_count = dev->mode_config.num_connector;
 	if (!max_conn_count) {
-		DRM_DEV_ERROR(dev->dev, "No connectors\n");
+		DRM_DEV_ERROR(dev->dev, "fbdev: No connectors\n");
 		return -EINVAL;
 	}
 
@@ -2850,13 +2850,13 @@ int drm_fb_helper_fbdev_setup(struct drm_device *dev,
 
 	ret = drm_fb_helper_init(dev, fb_helper, max_conn_count);
 	if (ret < 0) {
-		DRM_DEV_ERROR(dev->dev, "Failed to initialize fbdev helper\n");
+		DRM_DEV_ERROR(dev->dev, "fbdev: Failed to initialize (ret=%d)\n", ret);
 		return ret;
 	}
 
 	ret = drm_fb_helper_single_add_all_connectors(fb_helper);
 	if (ret < 0) {
-		DRM_DEV_ERROR(dev->dev, "Failed to add connectors\n");
+		DRM_DEV_ERROR(dev->dev, "fbdev: Failed to add connectors (ret=%d)\n", ret);
 		goto err_drm_fb_helper_fini;
 	}
 
@@ -2865,7 +2865,7 @@ int drm_fb_helper_fbdev_setup(struct drm_device *dev,
 
 	ret = drm_fb_helper_initial_config(fb_helper, preferred_bpp);
 	if (ret < 0) {
-		DRM_DEV_ERROR(dev->dev, "Failed to set fbdev configuration\n");
+		DRM_DEV_ERROR(dev->dev, "fbdev: Failed to set configuration (ret=%d)\n", ret);
 		goto err_drm_fb_helper_fini;
 	}
 
-- 
2.15.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 01/20] drm/fb-helper: Improve error reporting in setup

[Noralf Trønnes]

Den 08.09.2018 15.46, skrev Noralf Trønnes:
> Improve error reporting in drm_fb_helper_fbdev_setup() by printing the
> error code. This is useful for drivers that choose to not fall over just
> because fbdev doesen't work, but still wants clues to why it failed.
> This way they don't have to provide an error message themselves.
>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>   drivers/gpu/drm/drm_fb_helper.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)

Applied to drm-misc-next with Sam's ack, thanks.

Noralf.

> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 4b0dd20bccb8..2f865a046f6d 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -2842,7 +2842,7 @@ int drm_fb_helper_fbdev_setup(struct drm_device *dev,
>   	if (!max_conn_count)
>   		max_conn_count = dev->mode_config.num_connector;
>   	if (!max_conn_count) {
> -		DRM_DEV_ERROR(dev->dev, "No connectors\n");
> +		DRM_DEV_ERROR(dev->dev, "fbdev: No connectors\n");
>   		return -EINVAL;
>   	}
>   
> @@ -2850,13 +2850,13 @@ int drm_fb_helper_fbdev_setup(struct drm_device *dev,
>   
>   	ret = drm_fb_helper_init(dev, fb_helper, max_conn_count);
>   	if (ret < 0) {
> -		DRM_DEV_ERROR(dev->dev, "Failed to initialize fbdev helper\n");
> +		DRM_DEV_ERROR(dev->dev, "fbdev: Failed to initialize (ret=%d)\n", ret);
>   		return ret;
>   	}
>   
>   	ret = drm_fb_helper_single_add_all_connectors(fb_helper);
>   	if (ret < 0) {
> -		DRM_DEV_ERROR(dev->dev, "Failed to add connectors\n");
> +		DRM_DEV_ERROR(dev->dev, "fbdev: Failed to add connectors (ret=%d)\n", ret);
>   		goto err_drm_fb_helper_fini;
>   	}
>   
> @@ -2865,7 +2865,7 @@ int drm_fb_helper_fbdev_setup(struct drm_device *dev,
>   
>   	ret = drm_fb_helper_initial_config(fb_helper, preferred_bpp);
>   	if (ret < 0) {
> -		DRM_DEV_ERROR(dev->dev, "Failed to set fbdev configuration\n");
> +		DRM_DEV_ERROR(dev->dev, "fbdev: Failed to set configuration (ret=%d)\n", ret);
>   		goto err_drm_fb_helper_fini;
>   	}
>   

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 02/20] drm/arc: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

The CMA helper is already using the drm_fb_helper_generic_probe part of
the generic fbdev emulation. This patch makes full use of the generic
fbdev emulation by using its drm_client callbacks. This means that
drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
now handled by the emulation code. Additionally fbdev unregister happens
automatically on drm_dev_unregister().

The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
driver. This is done to highlight the fact that fbdev emulation is an
internal client that makes use of the driver, it is not part of the
driver as such. If fbdev setup fails, an error is printed, but the driver
succeeds probing.

Cc: Alexey Brodkin <abrodkin@synopsys.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/arc/arcpgu.h     |  4 ----
 drivers/gpu/drm/arc/arcpgu_drv.c | 33 +++------------------------------
 2 files changed, 3 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/arc/arcpgu.h b/drivers/gpu/drm/arc/arcpgu.h
index e8fcf3ab1d9a..90ef76b19f8a 100644
--- a/drivers/gpu/drm/arc/arcpgu.h
+++ b/drivers/gpu/drm/arc/arcpgu.h
@@ -20,7 +20,6 @@
 struct arcpgu_drm_private {
 	void __iomem		*regs;
 	struct clk		*clk;
-	struct drm_fbdev_cma	*fbdev;
 	struct drm_framebuffer	*fb;
 	struct drm_crtc		crtc;
 	struct drm_plane	*plane;
@@ -43,8 +42,5 @@ static inline u32 arc_pgu_read(struct arcpgu_drm_private *arcpgu,
 int arc_pgu_setup_crtc(struct drm_device *dev);
 int arcpgu_drm_hdmi_init(struct drm_device *drm, struct device_node *np);
 int arcpgu_drm_sim_init(struct drm_device *drm, struct device_node *np);
-struct drm_fbdev_cma *arcpgu_fbdev_cma_init(struct drm_device *dev,
-	unsigned int preferred_bpp, unsigned int num_crtc,
-	unsigned int max_conn_count);
 
 #endif
diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c
index f067de4e1e82..d3bc5a5cb4c2 100644
--- a/drivers/gpu/drm/arc/arcpgu_drv.c
+++ b/drivers/gpu/drm/arc/arcpgu_drv.c
@@ -17,6 +17,7 @@
 #include <linux/clk.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_fb_cma_helper.h>
+#include <drm/drm_fb_helper.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_atomic_helper.h>
@@ -25,16 +26,8 @@
 #include "arcpgu.h"
 #include "arcpgu_regs.h"
 
-static void arcpgu_fb_output_poll_changed(struct drm_device *dev)
-{
-	struct arcpgu_drm_private *arcpgu = dev->dev_private;
-
-	drm_fbdev_cma_hotplug_event(arcpgu->fbdev);
-}
-
 static const struct drm_mode_config_funcs arcpgu_drm_modecfg_funcs = {
 	.fb_create  = drm_gem_fb_create,
-	.output_poll_changed = arcpgu_fb_output_poll_changed,
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = drm_atomic_helper_commit,
 };
@@ -51,13 +44,6 @@ static void arcpgu_setup_mode_config(struct drm_device *drm)
 
 DEFINE_DRM_GEM_CMA_FOPS(arcpgu_drm_ops);
 
-static void arcpgu_lastclose(struct drm_device *drm)
-{
-	struct arcpgu_drm_private *arcpgu = drm->dev_private;
-
-	drm_fbdev_cma_restore_mode(arcpgu->fbdev);
-}
-
 static int arcpgu_load(struct drm_device *drm)
 {
 	struct platform_device *pdev = to_platform_device(drm->dev);
@@ -113,26 +99,12 @@ static int arcpgu_load(struct drm_device *drm)
 	drm_mode_config_reset(drm);
 	drm_kms_helper_poll_init(drm);
 
-	arcpgu->fbdev = drm_fbdev_cma_init(drm, 16,
-					   drm->mode_config.num_connector);
-	if (IS_ERR(arcpgu->fbdev)) {
-		ret = PTR_ERR(arcpgu->fbdev);
-		arcpgu->fbdev = NULL;
-		return -ENODEV;
-	}
-
 	platform_set_drvdata(pdev, drm);
 	return 0;
 }
 
 static int arcpgu_unload(struct drm_device *drm)
 {
-	struct arcpgu_drm_private *arcpgu = drm->dev_private;
-
-	if (arcpgu->fbdev) {
-		drm_fbdev_cma_fini(arcpgu->fbdev);
-		arcpgu->fbdev = NULL;
-	}
 	drm_kms_helper_poll_fini(drm);
 	drm_mode_config_cleanup(drm);
 
@@ -167,7 +139,6 @@ static int arcpgu_debugfs_init(struct drm_minor *minor)
 static struct drm_driver arcpgu_drm_driver = {
 	.driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME |
 			   DRIVER_ATOMIC,
-	.lastclose = arcpgu_lastclose,
 	.name = "arcpgu",
 	.desc = "ARC PGU Controller",
 	.date = "20160219",
@@ -210,6 +181,8 @@ static int arcpgu_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_unload;
 
+	drm_fbdev_generic_setup(drm, 16);
+
 	return 0;
 
 err_unload:
-- 
2.15.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/20] drm/arc: Use drm_fbdev_generic_setup()

[Alexey Brodkin]

Hi Noralf,

On Sat, 2018-09-08 at 15:46 +0200, Noralf Trønnes wrote:
> The CMA helper is already using the drm_fb_helper_generic_probe part of
> the generic fbdev emulation. This patch makes full use of the generic
> fbdev emulation by using its drm_client callbacks. This means that
> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
> now handled by the emulation code. Additionally fbdev unregister happens
> automatically on drm_dev_unregister().
> 
> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
> driver. This is done to highlight the fact that fbdev emulation is an
> internal client that makes use of the driver, it is not part of the
> driver as such. If fbdev setup fails, an error is printed, but the driver
> succeeds probing.
> 
> Cc: Alexey Brodkin <abrodkin@synopsys.com>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---

Thanks for making these improvements!
I'm wondering what is a base for these changes so that I may try it on
my board?

Preferably I'd like to get a link to the tag in your git tree with all
the prerequisites in place.

-Alexey
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/20] drm/arc: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

Den 10.09.2018 14.31, skrev Alexey Brodkin:
> Hi Noralf,
>
> On Sat, 2018-09-08 at 15:46 +0200, Noralf Trønnes wrote:
>> The CMA helper is already using the drm_fb_helper_generic_probe part of
>> the generic fbdev emulation. This patch makes full use of the generic
>> fbdev emulation by using its drm_client callbacks. This means that
>> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
>> now handled by the emulation code. Additionally fbdev unregister happens
>> automatically on drm_dev_unregister().
>>
>> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
>> driver. This is done to highlight the fact that fbdev emulation is an
>> internal client that makes use of the driver, it is not part of the
>> driver as such. If fbdev setup fails, an error is printed, but the driver
>> succeeds probing.
>>
>> Cc: Alexey Brodkin <abrodkin@synopsys.com>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
> Thanks for making these improvements!

Glad you like it.
Funny how a simple idea can turn into 6 months of work.

> I'm wondering what is a base for these changes so that I may try it on
> my board?
>
> Preferably I'd like to get a link to the tag in your git tree with all
> the prerequisites in place.

It's only this one patch to apply for you.
I replied to the cover letter with some more context.

Noralf.

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/20] drm/arc: Use drm_fbdev_generic_setup()

[Alexey Brodkin]

Hi Noralf,

On Sat, 2018-09-08 at 15:46 +0200, Noralf Trønnes wrote:
> The CMA helper is already using the drm_fb_helper_generic_probe part of
> the generic fbdev emulation. This patch makes full use of the generic
> fbdev emulation by using its drm_client callbacks. This means that
> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
> now handled by the emulation code. Additionally fbdev unregister happens
> automatically on drm_dev_unregister().
> 
> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
> driver. This is done to highlight the fact that fbdev emulation is an
> internal client that makes use of the driver, it is not part of the
> driver as such. If fbdev setup fails, an error is printed, but the driver
> succeeds probing.
> 
> Cc: Alexey Brodkin <abrodkin@synopsys.com>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

Thanks for doing that!
Quite some code removed from the driver and still FBDEV emulation works :)

Acked-by: Alexey Brodkin <abrodkin@synopsys.com>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/20] drm/arc: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

Den 28.09.2018 09.34, skrev Alexey Brodkin:
> Hi Noralf,
>
> On Sat, 2018-09-08 at 15:46 +0200, Noralf Trønnes wrote:
>> The CMA helper is already using the drm_fb_helper_generic_probe part of
>> the generic fbdev emulation. This patch makes full use of the generic
>> fbdev emulation by using its drm_client callbacks. This means that
>> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
>> now handled by the emulation code. Additionally fbdev unregister happens
>> automatically on drm_dev_unregister().
>>
>> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
>> driver. This is done to highlight the fact that fbdev emulation is an
>> internal client that makes use of the driver, it is not part of the
>> driver as such. If fbdev setup fails, an error is printed, but the driver
>> succeeds probing.
>>
>> Cc: Alexey Brodkin <abrodkin@synopsys.com>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> Thanks for doing that!
> Quite some code removed from the driver and still FBDEV emulation works :)

Yeah we're down to one line of code in the driver to get fbdev emulation
which is quite nice a think :D

Noralf.

> Acked-by: Alexey Brodkin <abrodkin@synopsys.com>
>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/20] drm/arc: Use drm_fbdev_generic_setup()

[Alexey Brodkin]

Hi Noralf,

On Fri, 2018-09-28 at 12:42 +0200, Noralf Trønnes wrote:
> Den 28.09.2018 09.34, skrev Alexey Brodkin:
> > Hi Noralf,
> > 
> > On Sat, 2018-09-08 at 15:46 +0200, Noralf Trønnes wrote:
> > > The CMA helper is already using the drm_fb_helper_generic_probe part of
> > > the generic fbdev emulation. This patch makes full use of the generic
> > > fbdev emulation by using its drm_client callbacks. This means that
> > > drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
> > > now handled by the emulation code. Additionally fbdev unregister happens
> > > automatically on drm_dev_unregister().
> > > 
> > > The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
> > > driver. This is done to highlight the fact that fbdev emulation is an
> > > internal client that makes use of the driver, it is not part of the
> > > driver as such. If fbdev setup fails, an error is printed, but the driver
> > > succeeds probing.
> > > 
> > > Cc: Alexey Brodkin <abrodkin@synopsys.com>
> > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> > 
> > Thanks for doing that!
> > Quite some code removed from the driver and still FBDEV emulation works :)
> 
> Yeah we're down to one line of code in the driver to get fbdev emulation
> which is quite nice a think :D

Exactly!

And just to not lose this nice improvement - do I need to pull this through
my tree or entire series will be merged in main DRM tree?

-Alexey
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/20] drm/arc: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

Den 01.10.2018 09.56, skrev Alexey Brodkin:
> Hi Noralf,
>
> On Fri, 2018-09-28 at 12:42 +0200, Noralf Trønnes wrote:
>> Den 28.09.2018 09.34, skrev Alexey Brodkin:
>>> Hi Noralf,
>>>
>>> On Sat, 2018-09-08 at 15:46 +0200, Noralf Trønnes wrote:
>>>> The CMA helper is already using the drm_fb_helper_generic_probe part of
>>>> the generic fbdev emulation. This patch makes full use of the generic
>>>> fbdev emulation by using its drm_client callbacks. This means that
>>>> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
>>>> now handled by the emulation code. Additionally fbdev unregister happens
>>>> automatically on drm_dev_unregister().
>>>>
>>>> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
>>>> driver. This is done to highlight the fact that fbdev emulation is an
>>>> internal client that makes use of the driver, it is not part of the
>>>> driver as such. If fbdev setup fails, an error is printed, but the driver
>>>> succeeds probing.
>>>>
>>>> Cc: Alexey Brodkin <abrodkin@synopsys.com>
>>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>>> Thanks for doing that!
>>> Quite some code removed from the driver and still FBDEV emulation works :)
>> Yeah we're down to one line of code in the driver to get fbdev emulation
>> which is quite nice a think :D
> Exactly!
>
> And just to not lose this nice improvement - do I need to pull this through
> my tree or entire series will be merged in main DRM tree?

I apply the patches to drm-misc-next when I get acks.
Unless the maintainer wants to take it himself.
So, yes I'll apply this one :-)

Noralf.

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/20] drm/arc: Use drm_fbdev_generic_setup()

[Alexey Brodkin]

Hi Noralf,

On Mon, 2018-10-01 at 14:05 +0200, Noralf Trønnes wrote:
> 
> Den 01.10.2018 09.56, skrev Alexey Brodkin:
> > Hi Noralf,
> > 
> > On Fri, 2018-09-28 at 12:42 +0200, Noralf Trønnes wrote:
> > > Den 28.09.2018 09.34, skrev Alexey Brodkin:
> > > > Hi Noralf,
> > > > 
> > > > On Sat, 2018-09-08 at 15:46 +0200, Noralf Trønnes wrote:
> > > > > The CMA helper is already using the drm_fb_helper_generic_probe part of
> > > > > the generic fbdev emulation. This patch makes full use of the generic
> > > > > fbdev emulation by using its drm_client callbacks. This means that
> > > > > drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
> > > > > now handled by the emulation code. Additionally fbdev unregister happens
> > > > > automatically on drm_dev_unregister().
> > > > > 
> > > > > The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
> > > > > driver. This is done to highlight the fact that fbdev emulation is an
> > > > > internal client that makes use of the driver, it is not part of the
> > > > > driver as such. If fbdev setup fails, an error is printed, but the driver
> > > > > succeeds probing.
> > > > > 
> > > > > Cc: Alexey Brodkin <abrodkin@synopsys.com>
> > > > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> > > > 
> > > > Thanks for doing that!
> > > > Quite some code removed from the driver and still FBDEV emulation works :)
> > > 
> > > Yeah we're down to one line of code in the driver to get fbdev emulation
> > > which is quite nice a think :D
> > 
> > Exactly!
> > 
> > And just to not lose this nice improvement - do I need to pull this through
> > my tree or entire series will be merged in main DRM tree?
> 
> I apply the patches to drm-misc-next when I get acks.
> Unless the maintainer wants to take it himself.
> So, yes I'll apply this one :-)

Cool! thanks for doing that!

-Alexey
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 03/20] drm/fsl-dcu: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

The CMA helper is already using the drm_fb_helper_generic_probe part of
the generic fbdev emulation. This patch makes full use of the generic
fbdev emulation by using its drm_client callbacks. This means that
drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
now handled by the emulation code. Additionally fbdev unregister happens
automatically on drm_dev_unregister().

The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
driver. This is done to highlight the fact that fbdev emulation is an
internal client that makes use of the driver, it is not part of the
driver as such. If fbdev setup fails, an error is printed, but the driver
succeeds probing.

Cc: Stefan Agner <stefan@agner.ch>
Cc: Alison Wang <alison.wang@nxp.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 25 +++----------------------
 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h |  1 -
 2 files changed, 3 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
index 80232321a244..15816141e5fb 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -26,6 +26,7 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_fb_cma_helper.h>
+#include <drm/drm_fb_helper.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_modeset_helper.h>
 
@@ -89,20 +90,11 @@ static int fsl_dcu_load(struct drm_device *dev, unsigned long flags)
 			"Invalid legacyfb_depth.  Defaulting to 24bpp\n");
 		legacyfb_depth = 24;
 	}
-	fsl_dev->fbdev = drm_fbdev_cma_init(dev, legacyfb_depth, 1);
-	if (IS_ERR(fsl_dev->fbdev)) {
-		ret = PTR_ERR(fsl_dev->fbdev);
-		fsl_dev->fbdev = NULL;
-		goto done;
-	}
 
 	return 0;
 done:
 	drm_kms_helper_poll_fini(dev);
 
-	if (fsl_dev->fbdev)
-		drm_fbdev_cma_fini(fsl_dev->fbdev);
-
 	drm_mode_config_cleanup(dev);
 	drm_irq_uninstall(dev);
 	dev->dev_private = NULL;
@@ -112,14 +104,9 @@ static int fsl_dcu_load(struct drm_device *dev, unsigned long flags)
 
 static void fsl_dcu_unload(struct drm_device *dev)
 {
-	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
-
 	drm_atomic_helper_shutdown(dev);
 	drm_kms_helper_poll_fini(dev);
 
-	if (fsl_dev->fbdev)
-		drm_fbdev_cma_fini(fsl_dev->fbdev);
-
 	drm_mode_config_cleanup(dev);
 	drm_irq_uninstall(dev);
 
@@ -147,19 +134,11 @@ static irqreturn_t fsl_dcu_drm_irq(int irq, void *arg)
 	return IRQ_HANDLED;
 }
 
-static void fsl_dcu_drm_lastclose(struct drm_device *dev)
-{
-	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
-
-	drm_fbdev_cma_restore_mode(fsl_dev->fbdev);
-}
-
 DEFINE_DRM_GEM_CMA_FOPS(fsl_dcu_drm_fops);
 
 static struct drm_driver fsl_dcu_drm_driver = {
 	.driver_features	= DRIVER_HAVE_IRQ | DRIVER_GEM | DRIVER_MODESET
 				| DRIVER_PRIME | DRIVER_ATOMIC,
-	.lastclose		= fsl_dcu_drm_lastclose,
 	.load			= fsl_dcu_load,
 	.unload			= fsl_dcu_unload,
 	.irq_handler		= fsl_dcu_drm_irq,
@@ -355,6 +334,8 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto unref;
 
+	drm_fbdev_generic_setup(drm, legacyfb_depth);
+
 	return 0;
 
 unref:
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
index 93bfb98012d4..cb87bb74cb87 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
@@ -191,7 +191,6 @@ struct fsl_dcu_drm_device {
 	/*protects hardware register*/
 	spinlock_t irq_lock;
 	struct drm_device *drm;
-	struct drm_fbdev_cma *fbdev;
 	struct drm_crtc crtc;
 	struct drm_encoder encoder;
 	struct fsl_dcu_drm_connector connector;
-- 
2.15.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 03/20] drm/fsl-dcu: Use drm_fbdev_generic_setup()

[Stefan Agner]

On 08.09.2018 15:46, Noralf Trønnes wrote:
> The CMA helper is already using the drm_fb_helper_generic_probe part of
> the generic fbdev emulation. This patch makes full use of the generic
> fbdev emulation by using its drm_client callbacks. This means that
> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
> now handled by the emulation code. Additionally fbdev unregister happens
> automatically on drm_dev_unregister().
> 
> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
> driver. This is done to highlight the fact that fbdev emulation is an
> internal client that makes use of the driver, it is not part of the
> driver as such. If fbdev setup fails, an error is printed, but the driver
> succeeds probing.

I think the last sentence is currently not implemented, I think it will
silently succeed probing. Can we add something like this, similar to
tinydrm-core.c?

	if (ret)
		DRM_ERROR("Failed to initialize fbdev: %d\n", ret);

--
Stefan

> 
> Cc: Stefan Agner <stefan@agner.ch>
> Cc: Alison Wang <alison.wang@nxp.com>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 25 +++----------------------
>  drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h |  1 -
>  2 files changed, 3 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> index 80232321a244..15816141e5fb 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> @@ -26,6 +26,7 @@
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_crtc_helper.h>
>  #include <drm/drm_fb_cma_helper.h>
> +#include <drm/drm_fb_helper.h>
>  #include <drm/drm_gem_cma_helper.h>
>  #include <drm/drm_modeset_helper.h>
>  
> @@ -89,20 +90,11 @@ static int fsl_dcu_load(struct drm_device *dev,
> unsigned long flags)
>  			"Invalid legacyfb_depth.  Defaulting to 24bpp\n");
>  		legacyfb_depth = 24;
>  	}
> -	fsl_dev->fbdev = drm_fbdev_cma_init(dev, legacyfb_depth, 1);
> -	if (IS_ERR(fsl_dev->fbdev)) {
> -		ret = PTR_ERR(fsl_dev->fbdev);
> -		fsl_dev->fbdev = NULL;
> -		goto done;
> -	}
>  
>  	return 0;
>  done:
>  	drm_kms_helper_poll_fini(dev);
>  
> -	if (fsl_dev->fbdev)
> -		drm_fbdev_cma_fini(fsl_dev->fbdev);
> -
>  	drm_mode_config_cleanup(dev);
>  	drm_irq_uninstall(dev);
>  	dev->dev_private = NULL;
> @@ -112,14 +104,9 @@ static int fsl_dcu_load(struct drm_device *dev,
> unsigned long flags)
>  
>  static void fsl_dcu_unload(struct drm_device *dev)
>  {
> -	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
> -
>  	drm_atomic_helper_shutdown(dev);
>  	drm_kms_helper_poll_fini(dev);
>  
> -	if (fsl_dev->fbdev)
> -		drm_fbdev_cma_fini(fsl_dev->fbdev);
> -
>  	drm_mode_config_cleanup(dev);
>  	drm_irq_uninstall(dev);
>  
> @@ -147,19 +134,11 @@ static irqreturn_t fsl_dcu_drm_irq(int irq, void *arg)
>  	return IRQ_HANDLED;
>  }
>  
> -static void fsl_dcu_drm_lastclose(struct drm_device *dev)
> -{
> -	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
> -
> -	drm_fbdev_cma_restore_mode(fsl_dev->fbdev);
> -}
> -
>  DEFINE_DRM_GEM_CMA_FOPS(fsl_dcu_drm_fops);
>  
>  static struct drm_driver fsl_dcu_drm_driver = {
>  	.driver_features	= DRIVER_HAVE_IRQ | DRIVER_GEM | DRIVER_MODESET
>  				| DRIVER_PRIME | DRIVER_ATOMIC,
> -	.lastclose		= fsl_dcu_drm_lastclose,
>  	.load			= fsl_dcu_load,
>  	.unload			= fsl_dcu_unload,
>  	.irq_handler		= fsl_dcu_drm_irq,
> @@ -355,6 +334,8 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		goto unref;
>  
> +	drm_fbdev_generic_setup(drm, legacyfb_depth);
> +
>  	return 0;
>  
>  unref:
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
> b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
> index 93bfb98012d4..cb87bb74cb87 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
> @@ -191,7 +191,6 @@ struct fsl_dcu_drm_device {
>  	/*protects hardware register*/
>  	spinlock_t irq_lock;
>  	struct drm_device *drm;
> -	struct drm_fbdev_cma *fbdev;
>  	struct drm_crtc crtc;
>  	struct drm_encoder encoder;
>  	struct fsl_dcu_drm_connector connector;
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 03/20] drm/fsl-dcu: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

Den 27.09.2018 23.08, skrev Stefan Agner:
> On 08.09.2018 15:46, Noralf Trønnes wrote:
>> The CMA helper is already using the drm_fb_helper_generic_probe part of
>> the generic fbdev emulation. This patch makes full use of the generic
>> fbdev emulation by using its drm_client callbacks. This means that
>> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
>> now handled by the emulation code. Additionally fbdev unregister happens
>> automatically on drm_dev_unregister().
>>
>> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
>> driver. This is done to highlight the fact that fbdev emulation is an
>> internal client that makes use of the driver, it is not part of the
>> driver as such. If fbdev setup fails, an error is printed, but the driver
>> succeeds probing.
> I think the last sentence is currently not implemented, I think it will
> silently succeed probing. Can we add something like this, similar to
> tinydrm-core.c?
>
> 	if (ret)
> 		DRM_ERROR("Failed to initialize fbdev: %d\n", ret);

It was implemented in the first patch in the series, now applied:

drm/fb-helper: Improve error reporting in setup
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=6129369a96183f28c7912dfd37cb5869433aa904

Noralf.

> --
> Stefan
>
>> Cc: Stefan Agner <stefan@agner.ch>
>> Cc: Alison Wang <alison.wang@nxp.com>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>>   drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 25 +++----------------------
>>   drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h |  1 -
>>   2 files changed, 3 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>> b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>> index 80232321a244..15816141e5fb 100644
>> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>> @@ -26,6 +26,7 @@
>>   #include <drm/drm_atomic_helper.h>
>>   #include <drm/drm_crtc_helper.h>
>>   #include <drm/drm_fb_cma_helper.h>
>> +#include <drm/drm_fb_helper.h>
>>   #include <drm/drm_gem_cma_helper.h>
>>   #include <drm/drm_modeset_helper.h>
>>   
>> @@ -89,20 +90,11 @@ static int fsl_dcu_load(struct drm_device *dev,
>> unsigned long flags)
>>   			"Invalid legacyfb_depth.  Defaulting to 24bpp\n");
>>   		legacyfb_depth = 24;
>>   	}
>> -	fsl_dev->fbdev = drm_fbdev_cma_init(dev, legacyfb_depth, 1);
>> -	if (IS_ERR(fsl_dev->fbdev)) {
>> -		ret = PTR_ERR(fsl_dev->fbdev);
>> -		fsl_dev->fbdev = NULL;
>> -		goto done;
>> -	}
>>   
>>   	return 0;
>>   done:
>>   	drm_kms_helper_poll_fini(dev);
>>   
>> -	if (fsl_dev->fbdev)
>> -		drm_fbdev_cma_fini(fsl_dev->fbdev);
>> -
>>   	drm_mode_config_cleanup(dev);
>>   	drm_irq_uninstall(dev);
>>   	dev->dev_private = NULL;
>> @@ -112,14 +104,9 @@ static int fsl_dcu_load(struct drm_device *dev,
>> unsigned long flags)
>>   
>>   static void fsl_dcu_unload(struct drm_device *dev)
>>   {
>> -	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
>> -
>>   	drm_atomic_helper_shutdown(dev);
>>   	drm_kms_helper_poll_fini(dev);
>>   
>> -	if (fsl_dev->fbdev)
>> -		drm_fbdev_cma_fini(fsl_dev->fbdev);
>> -
>>   	drm_mode_config_cleanup(dev);
>>   	drm_irq_uninstall(dev);
>>   
>> @@ -147,19 +134,11 @@ static irqreturn_t fsl_dcu_drm_irq(int irq, void *arg)
>>   	return IRQ_HANDLED;
>>   }
>>   
>> -static void fsl_dcu_drm_lastclose(struct drm_device *dev)
>> -{
>> -	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
>> -
>> -	drm_fbdev_cma_restore_mode(fsl_dev->fbdev);
>> -}
>> -
>>   DEFINE_DRM_GEM_CMA_FOPS(fsl_dcu_drm_fops);
>>   
>>   static struct drm_driver fsl_dcu_drm_driver = {
>>   	.driver_features	= DRIVER_HAVE_IRQ | DRIVER_GEM | DRIVER_MODESET
>>   				| DRIVER_PRIME | DRIVER_ATOMIC,
>> -	.lastclose		= fsl_dcu_drm_lastclose,
>>   	.load			= fsl_dcu_load,
>>   	.unload			= fsl_dcu_unload,
>>   	.irq_handler		= fsl_dcu_drm_irq,
>> @@ -355,6 +334,8 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev)
>>   	if (ret < 0)
>>   		goto unref;
>>   
>> +	drm_fbdev_generic_setup(drm, legacyfb_depth);
>> +
>>   	return 0;
>>   
>>   unref:
>> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
>> b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
>> index 93bfb98012d4..cb87bb74cb87 100644
>> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
>> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
>> @@ -191,7 +191,6 @@ struct fsl_dcu_drm_device {
>>   	/*protects hardware register*/
>>   	spinlock_t irq_lock;
>>   	struct drm_device *drm;
>> -	struct drm_fbdev_cma *fbdev;
>>   	struct drm_crtc crtc;
>>   	struct drm_encoder encoder;
>>   	struct fsl_dcu_drm_connector connector;

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 03/20] drm/fsl-dcu: Use drm_fbdev_generic_setup()

[Stefan Agner]

On 27.09.2018 23:23, Noralf Trønnes wrote:
> Den 27.09.2018 23.08, skrev Stefan Agner:
>> On 08.09.2018 15:46, Noralf Trønnes wrote:
>>> The CMA helper is already using the drm_fb_helper_generic_probe part of
>>> the generic fbdev emulation. This patch makes full use of the generic
>>> fbdev emulation by using its drm_client callbacks. This means that
>>> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
>>> now handled by the emulation code. Additionally fbdev unregister happens
>>> automatically on drm_dev_unregister().
>>>
>>> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
>>> driver. This is done to highlight the fact that fbdev emulation is an
>>> internal client that makes use of the driver, it is not part of the
>>> driver as such. If fbdev setup fails, an error is printed, but the driver
>>> succeeds probing.
>> I think the last sentence is currently not implemented, I think it will
>> silently succeed probing. Can we add something like this, similar to
>> tinydrm-core.c?
>>
>> 	if (ret)
>> 		DRM_ERROR("Failed to initialize fbdev: %d\n", ret);
> 
> It was implemented in the first patch in the series, now applied:
> 
> drm/fb-helper: Improve error reporting in setup
> https://cgit.freedesktop.org/drm/drm-misc/commit/?id=6129369a96183f28c7912dfd37cb5869433aa904

That patch adds error reporting for drm_fb_helper_fbdev_setup().

But you are using drm_fbdev_generic_setup() here...

--
Stefan

> 
> Noralf.
> 
>> --
>> Stefan
>>
>>> Cc: Stefan Agner <stefan@agner.ch>
>>> Cc: Alison Wang <alison.wang@nxp.com>
>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>>> ---
>>>   drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 25 +++----------------------
>>>   drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h |  1 -
>>>   2 files changed, 3 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>>> b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>>> index 80232321a244..15816141e5fb 100644
>>> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>>> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>>> @@ -26,6 +26,7 @@
>>>   #include <drm/drm_atomic_helper.h>
>>>   #include <drm/drm_crtc_helper.h>
>>>   #include <drm/drm_fb_cma_helper.h>
>>> +#include <drm/drm_fb_helper.h>
>>>   #include <drm/drm_gem_cma_helper.h>
>>>   #include <drm/drm_modeset_helper.h>
>>>   @@ -89,20 +90,11 @@ static int fsl_dcu_load(struct drm_device *dev,
>>> unsigned long flags)
>>>   			"Invalid legacyfb_depth.  Defaulting to 24bpp\n");
>>>   		legacyfb_depth = 24;
>>>   	}
>>> -	fsl_dev->fbdev = drm_fbdev_cma_init(dev, legacyfb_depth, 1);
>>> -	if (IS_ERR(fsl_dev->fbdev)) {
>>> -		ret = PTR_ERR(fsl_dev->fbdev);
>>> -		fsl_dev->fbdev = NULL;
>>> -		goto done;
>>> -	}
>>>     	return 0;
>>>   done:
>>>   	drm_kms_helper_poll_fini(dev);
>>>   -	if (fsl_dev->fbdev)
>>> -		drm_fbdev_cma_fini(fsl_dev->fbdev);
>>> -
>>>   	drm_mode_config_cleanup(dev);
>>>   	drm_irq_uninstall(dev);
>>>   	dev->dev_private = NULL;
>>> @@ -112,14 +104,9 @@ static int fsl_dcu_load(struct drm_device *dev,
>>> unsigned long flags)
>>>     static void fsl_dcu_unload(struct drm_device *dev)
>>>   {
>>> -	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
>>> -
>>>   	drm_atomic_helper_shutdown(dev);
>>>   	drm_kms_helper_poll_fini(dev);
>>>   -	if (fsl_dev->fbdev)
>>> -		drm_fbdev_cma_fini(fsl_dev->fbdev);
>>> -
>>>   	drm_mode_config_cleanup(dev);
>>>   	drm_irq_uninstall(dev);
>>>   @@ -147,19 +134,11 @@ static irqreturn_t fsl_dcu_drm_irq(int irq, void *arg)
>>>   	return IRQ_HANDLED;
>>>   }
>>>   -static void fsl_dcu_drm_lastclose(struct drm_device *dev)
>>> -{
>>> -	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
>>> -
>>> -	drm_fbdev_cma_restore_mode(fsl_dev->fbdev);
>>> -}
>>> -
>>>   DEFINE_DRM_GEM_CMA_FOPS(fsl_dcu_drm_fops);
>>>     static struct drm_driver fsl_dcu_drm_driver = {
>>>   	.driver_features	= DRIVER_HAVE_IRQ | DRIVER_GEM | DRIVER_MODESET
>>>   				| DRIVER_PRIME | DRIVER_ATOMIC,
>>> -	.lastclose		= fsl_dcu_drm_lastclose,
>>>   	.load			= fsl_dcu_load,
>>>   	.unload			= fsl_dcu_unload,
>>>   	.irq_handler		= fsl_dcu_drm_irq,
>>> @@ -355,6 +334,8 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev)
>>>   	if (ret < 0)
>>>   		goto unref;
>>>   +	drm_fbdev_generic_setup(drm, legacyfb_depth);
>>> +
>>>   	return 0;
>>>     unref:
>>> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
>>> b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
>>> index 93bfb98012d4..cb87bb74cb87 100644
>>> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
>>> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
>>> @@ -191,7 +191,6 @@ struct fsl_dcu_drm_device {
>>>   	/*protects hardware register*/
>>>   	spinlock_t irq_lock;
>>>   	struct drm_device *drm;
>>> -	struct drm_fbdev_cma *fbdev;
>>>   	struct drm_crtc crtc;
>>>   	struct drm_encoder encoder;
>>>   	struct fsl_dcu_drm_connector connector;
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 03/20] drm/fsl-dcu: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

Den 28.09.2018 16.11, skrev Stefan Agner:
> On 27.09.2018 23:23, Noralf Trønnes wrote:
>> Den 27.09.2018 23.08, skrev Stefan Agner:
>>> On 08.09.2018 15:46, Noralf Trønnes wrote:
>>>> The CMA helper is already using the drm_fb_helper_generic_probe part of
>>>> the generic fbdev emulation. This patch makes full use of the generic
>>>> fbdev emulation by using its drm_client callbacks. This means that
>>>> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
>>>> now handled by the emulation code. Additionally fbdev unregister happens
>>>> automatically on drm_dev_unregister().
>>>>
>>>> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
>>>> driver. This is done to highlight the fact that fbdev emulation is an
>>>> internal client that makes use of the driver, it is not part of the
>>>> driver as such. If fbdev setup fails, an error is printed, but the driver
>>>> succeeds probing.
>>> I think the last sentence is currently not implemented, I think it will
>>> silently succeed probing. Can we add something like this, similar to
>>> tinydrm-core.c?
>>>
>>> 	if (ret)
>>> 		DRM_ERROR("Failed to initialize fbdev: %d\n", ret);
>> It was implemented in the first patch in the series, now applied:
>>
>> drm/fb-helper: Improve error reporting in setup
>> https://cgit.freedesktop.org/drm/drm-misc/commit/?id=6129369a96183f28c7912dfd37cb5869433aa904
> That patch adds error reporting for drm_fb_helper_fbdev_setup().
>
> But you are using drm_fbdev_generic_setup() here...

Yep, this is the callchain:

drm_fbdev_generic_setup()
   -> drm_fbdev_client_hotplug()
     -> drm_fb_helper_fbdev_setup()

The benefit of having setup in the hotplug callback is that now fbdev is
supported in the case where there are no connectors at driver load.

Noralf.

> --
> Stefan
>
>> Noralf.
>>
>>> --
>>> Stefan
>>>
>>>> Cc: Stefan Agner <stefan@agner.ch>
>>>> Cc: Alison Wang <alison.wang@nxp.com>
>>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>>>> ---
>>>>    drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 25 +++----------------------
>>>>    drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h |  1 -
>>>>    2 files changed, 3 insertions(+), 23 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>>>> b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>>>> index 80232321a244..15816141e5fb 100644
>>>> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>>>> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>>>> @@ -26,6 +26,7 @@
>>>>    #include <drm/drm_atomic_helper.h>
>>>>    #include <drm/drm_crtc_helper.h>
>>>>    #include <drm/drm_fb_cma_helper.h>
>>>> +#include <drm/drm_fb_helper.h>
>>>>    #include <drm/drm_gem_cma_helper.h>
>>>>    #include <drm/drm_modeset_helper.h>
>>>>    @@ -89,20 +90,11 @@ static int fsl_dcu_load(struct drm_device *dev,
>>>> unsigned long flags)
>>>>    			"Invalid legacyfb_depth.  Defaulting to 24bpp\n");
>>>>    		legacyfb_depth = 24;
>>>>    	}
>>>> -	fsl_dev->fbdev = drm_fbdev_cma_init(dev, legacyfb_depth, 1);
>>>> -	if (IS_ERR(fsl_dev->fbdev)) {
>>>> -		ret = PTR_ERR(fsl_dev->fbdev);
>>>> -		fsl_dev->fbdev = NULL;
>>>> -		goto done;
>>>> -	}
>>>>      	return 0;
>>>>    done:
>>>>    	drm_kms_helper_poll_fini(dev);
>>>>    -	if (fsl_dev->fbdev)
>>>> -		drm_fbdev_cma_fini(fsl_dev->fbdev);
>>>> -
>>>>    	drm_mode_config_cleanup(dev);
>>>>    	drm_irq_uninstall(dev);
>>>>    	dev->dev_private = NULL;
>>>> @@ -112,14 +104,9 @@ static int fsl_dcu_load(struct drm_device *dev,
>>>> unsigned long flags)
>>>>      static void fsl_dcu_unload(struct drm_device *dev)
>>>>    {
>>>> -	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
>>>> -
>>>>    	drm_atomic_helper_shutdown(dev);
>>>>    	drm_kms_helper_poll_fini(dev);
>>>>    -	if (fsl_dev->fbdev)
>>>> -		drm_fbdev_cma_fini(fsl_dev->fbdev);
>>>> -
>>>>    	drm_mode_config_cleanup(dev);
>>>>    	drm_irq_uninstall(dev);
>>>>    @@ -147,19 +134,11 @@ static irqreturn_t fsl_dcu_drm_irq(int irq, void *arg)
>>>>    	return IRQ_HANDLED;
>>>>    }
>>>>    -static void fsl_dcu_drm_lastclose(struct drm_device *dev)
>>>> -{
>>>> -	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
>>>> -
>>>> -	drm_fbdev_cma_restore_mode(fsl_dev->fbdev);
>>>> -}
>>>> -
>>>>    DEFINE_DRM_GEM_CMA_FOPS(fsl_dcu_drm_fops);
>>>>      static struct drm_driver fsl_dcu_drm_driver = {
>>>>    	.driver_features	= DRIVER_HAVE_IRQ | DRIVER_GEM | DRIVER_MODESET
>>>>    				| DRIVER_PRIME | DRIVER_ATOMIC,
>>>> -	.lastclose		= fsl_dcu_drm_lastclose,
>>>>    	.load			= fsl_dcu_load,
>>>>    	.unload			= fsl_dcu_unload,
>>>>    	.irq_handler		= fsl_dcu_drm_irq,
>>>> @@ -355,6 +334,8 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev)
>>>>    	if (ret < 0)
>>>>    		goto unref;
>>>>    +	drm_fbdev_generic_setup(drm, legacyfb_depth);
>>>> +
>>>>    	return 0;
>>>>      unref:
>>>> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
>>>> b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
>>>> index 93bfb98012d4..cb87bb74cb87 100644
>>>> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
>>>> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
>>>> @@ -191,7 +191,6 @@ struct fsl_dcu_drm_device {
>>>>    	/*protects hardware register*/
>>>>    	spinlock_t irq_lock;
>>>>    	struct drm_device *drm;
>>>> -	struct drm_fbdev_cma *fbdev;
>>>>    	struct drm_crtc crtc;
>>>>    	struct drm_encoder encoder;
>>>>    	struct fsl_dcu_drm_connector connector;

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 03/20] drm/fsl-dcu: Use drm_fbdev_generic_setup()

[Stefan Agner]

On 28.09.2018 16:57, Noralf Trønnes wrote:
> Den 28.09.2018 16.11, skrev Stefan Agner:
>> On 27.09.2018 23:23, Noralf Trønnes wrote:
>>> Den 27.09.2018 23.08, skrev Stefan Agner:
>>>> On 08.09.2018 15:46, Noralf Trønnes wrote:
>>>>> The CMA helper is already using the drm_fb_helper_generic_probe part of
>>>>> the generic fbdev emulation. This patch makes full use of the generic
>>>>> fbdev emulation by using its drm_client callbacks. This means that
>>>>> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
>>>>> now handled by the emulation code. Additionally fbdev unregister happens
>>>>> automatically on drm_dev_unregister().
>>>>>
>>>>> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
>>>>> driver. This is done to highlight the fact that fbdev emulation is an
>>>>> internal client that makes use of the driver, it is not part of the
>>>>> driver as such. If fbdev setup fails, an error is printed, but the driver
>>>>> succeeds probing.
>>>> I think the last sentence is currently not implemented, I think it will
>>>> silently succeed probing. Can we add something like this, similar to
>>>> tinydrm-core.c?
>>>>
>>>> 	if (ret)
>>>> 		DRM_ERROR("Failed to initialize fbdev: %d\n", ret);
>>> It was implemented in the first patch in the series, now applied:
>>>
>>> drm/fb-helper: Improve error reporting in setup
>>> https://cgit.freedesktop.org/drm/drm-misc/commit/?id=6129369a96183f28c7912dfd37cb5869433aa904
>> That patch adds error reporting for drm_fb_helper_fbdev_setup().
>>
>> But you are using drm_fbdev_generic_setup() here...
> 
> Yep, this is the callchain:
> 
> drm_fbdev_generic_setup()
>   -> drm_fbdev_client_hotplug()
>     -> drm_fb_helper_fbdev_setup()
> 
> The benefit of having setup in the hotplug callback is that now fbdev is
> supported in the case where there are no connectors at driver load.
> 

Oh ok, I see.

Then, this patch looks good for me:

Acked-by: Stefan Agner <stefan@agner.ch>

--
Stefan

> Noralf.
> 
>> --
>> Stefan
>>
>>> Noralf.
>>>
>>>> --
>>>> Stefan
>>>>
>>>>> Cc: Stefan Agner <stefan@agner.ch>
>>>>> Cc: Alison Wang <alison.wang@nxp.com>
>>>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>>>>> ---
>>>>>    drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 25 +++----------------------
>>>>>    drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h |  1 -
>>>>>    2 files changed, 3 insertions(+), 23 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>>>>> b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>>>>> index 80232321a244..15816141e5fb 100644
>>>>> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>>>>> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
>>>>> @@ -26,6 +26,7 @@
>>>>>    #include <drm/drm_atomic_helper.h>
>>>>>    #include <drm/drm_crtc_helper.h>
>>>>>    #include <drm/drm_fb_cma_helper.h>
>>>>> +#include <drm/drm_fb_helper.h>
>>>>>    #include <drm/drm_gem_cma_helper.h>
>>>>>    #include <drm/drm_modeset_helper.h>
>>>>>    @@ -89,20 +90,11 @@ static int fsl_dcu_load(struct drm_device *dev,
>>>>> unsigned long flags)
>>>>>    			"Invalid legacyfb_depth.  Defaulting to 24bpp\n");
>>>>>    		legacyfb_depth = 24;
>>>>>    	}
>>>>> -	fsl_dev->fbdev = drm_fbdev_cma_init(dev, legacyfb_depth, 1);
>>>>> -	if (IS_ERR(fsl_dev->fbdev)) {
>>>>> -		ret = PTR_ERR(fsl_dev->fbdev);
>>>>> -		fsl_dev->fbdev = NULL;
>>>>> -		goto done;
>>>>> -	}
>>>>>      	return 0;
>>>>>    done:
>>>>>    	drm_kms_helper_poll_fini(dev);
>>>>>    -	if (fsl_dev->fbdev)
>>>>> -		drm_fbdev_cma_fini(fsl_dev->fbdev);
>>>>> -
>>>>>    	drm_mode_config_cleanup(dev);
>>>>>    	drm_irq_uninstall(dev);
>>>>>    	dev->dev_private = NULL;
>>>>> @@ -112,14 +104,9 @@ static int fsl_dcu_load(struct drm_device *dev,
>>>>> unsigned long flags)
>>>>>      static void fsl_dcu_unload(struct drm_device *dev)
>>>>>    {
>>>>> -	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
>>>>> -
>>>>>    	drm_atomic_helper_shutdown(dev);
>>>>>    	drm_kms_helper_poll_fini(dev);
>>>>>    -	if (fsl_dev->fbdev)
>>>>> -		drm_fbdev_cma_fini(fsl_dev->fbdev);
>>>>> -
>>>>>    	drm_mode_config_cleanup(dev);
>>>>>    	drm_irq_uninstall(dev);
>>>>>    @@ -147,19 +134,11 @@ static irqreturn_t fsl_dcu_drm_irq(int irq, void *arg)
>>>>>    	return IRQ_HANDLED;
>>>>>    }
>>>>>    -static void fsl_dcu_drm_lastclose(struct drm_device *dev)
>>>>> -{
>>>>> -	struct fsl_dcu_drm_device *fsl_dev = dev->dev_private;
>>>>> -
>>>>> -	drm_fbdev_cma_restore_mode(fsl_dev->fbdev);
>>>>> -}
>>>>> -
>>>>>    DEFINE_DRM_GEM_CMA_FOPS(fsl_dcu_drm_fops);
>>>>>      static struct drm_driver fsl_dcu_drm_driver = {
>>>>>    	.driver_features	= DRIVER_HAVE_IRQ | DRIVER_GEM | DRIVER_MODESET
>>>>>    				| DRIVER_PRIME | DRIVER_ATOMIC,
>>>>> -	.lastclose		= fsl_dcu_drm_lastclose,
>>>>>    	.load			= fsl_dcu_load,
>>>>>    	.unload			= fsl_dcu_unload,
>>>>>    	.irq_handler		= fsl_dcu_drm_irq,
>>>>> @@ -355,6 +334,8 @@ static int fsl_dcu_drm_probe(struct platform_device *pdev)
>>>>>    	if (ret < 0)
>>>>>    		goto unref;
>>>>>    +	drm_fbdev_generic_setup(drm, legacyfb_depth);
>>>>> +
>>>>>    	return 0;
>>>>>      unref:
>>>>> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
>>>>> b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
>>>>> index 93bfb98012d4..cb87bb74cb87 100644
>>>>> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
>>>>> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h
>>>>> @@ -191,7 +191,6 @@ struct fsl_dcu_drm_device {
>>>>>    	/*protects hardware register*/
>>>>>    	spinlock_t irq_lock;
>>>>>    	struct drm_device *drm;
>>>>> -	struct drm_fbdev_cma *fbdev;
>>>>>    	struct drm_crtc crtc;
>>>>>    	struct drm_encoder encoder;
>>>>>    	struct fsl_dcu_drm_connector connector;
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 04/20] drm/hisilicon/kirin: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

The CMA helper is already using the drm_fb_helper_generic_probe part of
the generic fbdev emulation. This patch makes full use of the generic
fbdev emulation by using its drm_client callbacks. This means that
drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
now handled by the emulation code. Additionally fbdev unregister happens
automatically on drm_dev_unregister().

The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
driver. This is done to highlight the fact that fbdev emulation is an
internal client that makes use of the driver, it is not part of the
driver as such. If fbdev setup fails, an error is printed, but the driver
succeeds probing.

struct kirin_drm_private can be removed now that driver doesn't have to
store the fbdev pointer.

Cc: Xinliang Liu <z.liuxinliang@hisilicon.com>
Cc: Rongrong Zou <zourongrong@gmail.com>
Cc: Xinwei Kong <kong.kongxinwei@hisilicon.com>
Cc: Chen Feng <puck.chen@hisilicon.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 38 ++-----------------------
 drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h |  4 ---
 2 files changed, 3 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
index ddb0403f1975..db11566705ba 100644
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
@@ -22,6 +22,7 @@
 #include <drm/drmP.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_fb_cma_helper.h>
+#include <drm/drm_fb_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc_helper.h>
@@ -33,32 +34,15 @@ static struct kirin_dc_ops *dc_ops;
 
 static int kirin_drm_kms_cleanup(struct drm_device *dev)
 {
-	struct kirin_drm_private *priv = dev->dev_private;
-
-	if (priv->fbdev) {
-		drm_fbdev_cma_fini(priv->fbdev);
-		priv->fbdev = NULL;
-	}
-
 	drm_kms_helper_poll_fini(dev);
 	dc_ops->cleanup(to_platform_device(dev->dev));
 	drm_mode_config_cleanup(dev);
-	devm_kfree(dev->dev, priv);
-	dev->dev_private = NULL;
 
 	return 0;
 }
 
-static void kirin_fbdev_output_poll_changed(struct drm_device *dev)
-{
-	struct kirin_drm_private *priv = dev->dev_private;
-
-	drm_fbdev_cma_hotplug_event(priv->fbdev);
-}
-
 static const struct drm_mode_config_funcs kirin_drm_mode_config_funcs = {
 	.fb_create = drm_gem_fb_create,
-	.output_poll_changed = kirin_fbdev_output_poll_changed,
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = drm_atomic_helper_commit,
 };
@@ -76,14 +60,8 @@ static void kirin_drm_mode_config_init(struct drm_device *dev)
 
 static int kirin_drm_kms_init(struct drm_device *dev)
 {
-	struct kirin_drm_private *priv;
 	int ret;
 
-	priv = devm_kzalloc(dev->dev, sizeof(*priv), GFP_KERNEL);
-	if (!priv)
-		return -ENOMEM;
-
-	dev->dev_private = priv;
 	dev_set_drvdata(dev->dev, dev);
 
 	/* dev->mode_config initialization */
@@ -117,26 +95,14 @@ static int kirin_drm_kms_init(struct drm_device *dev)
 	/* init kms poll for handling hpd */
 	drm_kms_helper_poll_init(dev);
 
-	priv->fbdev = drm_fbdev_cma_init(dev, 32,
-					 dev->mode_config.num_connector);
-
-	if (IS_ERR(priv->fbdev)) {
-		DRM_ERROR("failed to initialize fbdev.\n");
-		ret = PTR_ERR(priv->fbdev);
-		goto err_cleanup_poll;
-	}
 	return 0;
 
-err_cleanup_poll:
-	drm_kms_helper_poll_fini(dev);
 err_unbind_all:
 	component_unbind_all(dev->dev, dev);
 err_dc_cleanup:
 	dc_ops->cleanup(to_platform_device(dev->dev));
 err_mode_config_cleanup:
 	drm_mode_config_cleanup(dev);
-	devm_kfree(dev->dev, priv);
-	dev->dev_private = NULL;
 
 	return ret;
 }
@@ -199,6 +165,8 @@ static int kirin_drm_bind(struct device *dev)
 	if (ret)
 		goto err_kms_cleanup;
 
+	drm_fbdev_generic_setup(drm_dev, 32);
+
 	return 0;
 
 err_kms_cleanup:
diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h
index 56cb62df065c..ad027d1cc826 100644
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h
@@ -19,10 +19,6 @@ struct kirin_dc_ops {
 	void (*cleanup)(struct platform_device *pdev);
 };
 
-struct kirin_drm_private {
-	struct drm_fbdev_cma *fbdev;
-};
-
 extern const struct kirin_dc_ops ade_dc_ops;
 
 #endif /* __KIRIN_DRM_DRV_H__ */
-- 
2.15.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 05/20] drm/meson: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

The CMA helper is already using the drm_fb_helper_generic_probe part of
the generic fbdev emulation. This patch makes full use of the generic
fbdev emulation by using its drm_client callbacks. This means that
drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
now handled by the emulation code. Additionally fbdev unregister happens
automatically on drm_dev_unregister().

The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
driver. This is done to highlight the fact that fbdev emulation is an
internal client that makes use of the driver, it is not part of the
driver as such. If fbdev setup fails, an error is printed, but the driver
succeeds probing.

Cc: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/meson/meson_drv.c | 19 ++-----------------
 drivers/gpu/drm/meson/meson_drv.h |  1 -
 2 files changed, 2 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
index d3443125e661..348b5a198b9d 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -68,15 +68,7 @@
  * - Powering Up HDMI controller and PHY
  */
 
-static void meson_fb_output_poll_changed(struct drm_device *dev)
-{
-	struct meson_drm *priv = dev->dev_private;
-
-	drm_fbdev_cma_hotplug_event(priv->fbdev);
-}
-
 static const struct drm_mode_config_funcs meson_mode_config_funcs = {
-	.output_poll_changed = meson_fb_output_poll_changed,
 	.atomic_check        = drm_atomic_helper_check,
 	.atomic_commit       = drm_atomic_helper_commit,
 	.fb_create           = drm_gem_fb_create,
@@ -282,13 +274,6 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
 
 	drm_mode_config_reset(drm);
 
-	priv->fbdev = drm_fbdev_cma_init(drm, 32,
-					 drm->mode_config.num_connector);
-	if (IS_ERR(priv->fbdev)) {
-		ret = PTR_ERR(priv->fbdev);
-		goto free_drm;
-	}
-
 	drm_kms_helper_poll_init(drm);
 
 	platform_set_drvdata(pdev, priv);
@@ -297,6 +282,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
 	if (ret)
 		goto free_drm;
 
+	drm_fbdev_generic_setup(drm, 32);
+
 	return 0;
 
 free_drm:
@@ -313,11 +300,9 @@ static int meson_drv_bind(struct device *dev)
 static void meson_drv_unbind(struct device *dev)
 {
 	struct drm_device *drm = dev_get_drvdata(dev);
-	struct meson_drm *priv = drm->dev_private;
 
 	drm_dev_unregister(drm);
 	drm_kms_helper_poll_fini(drm);
-	drm_fbdev_cma_fini(priv->fbdev);
 	drm_mode_config_cleanup(drm);
 	drm_dev_put(drm);
 
diff --git a/drivers/gpu/drm/meson/meson_drv.h b/drivers/gpu/drm/meson/meson_drv.h
index 8450d6ac8c9b..aab96260da9f 100644
--- a/drivers/gpu/drm/meson/meson_drv.h
+++ b/drivers/gpu/drm/meson/meson_drv.h
@@ -33,7 +33,6 @@ struct meson_drm {
 
 	struct drm_device *drm;
 	struct drm_crtc *crtc;
-	struct drm_fbdev_cma *fbdev;
 	struct drm_plane *primary_plane;
 
 	/* Components Data */
-- 
2.15.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 05/20] drm/meson: Use drm_fbdev_generic_setup()

[Neil Armstrong]

Hi Noralf,

On 08/09/2018 15:46, Noralf Trønnes wrote:
> The CMA helper is already using the drm_fb_helper_generic_probe part of
> the generic fbdev emulation. This patch makes full use of the generic
> fbdev emulation by using its drm_client callbacks. This means that
> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
> now handled by the emulation code. Additionally fbdev unregister happens
> automatically on drm_dev_unregister().
> 
> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
> driver. This is done to highlight the fact that fbdev emulation is an
> internal client that makes use of the driver, it is not part of the
> driver as such. If fbdev setup fails, an error is printed, but the driver
> succeeds probing.


I can't really ack/nack this move, on one side it's great to have a generic fbdev emulation
instead of the old fbdev code, but on the other side the Amlogic platform (like allwinner, samsung, rockchip, ...)
still relies on an Fbdev variant of the libMali that uses smem_start...

I know it's dirty and fbdev based code should be legacy now, but I consider it like an user-space breakage...

I'll be happy if ARM provided it's Mali vendors a Fbdev libMali that could use FBINFO_HIDE_SMEM_START and allocate BO's
from the DRM driver, it won't be the case and it will never be the case until the Lima projects finalizes.

So for me it's a no-go until Lima lands upstream in Linux *and* Mesa.

Neil

> 
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  drivers/gpu/drm/meson/meson_drv.c | 19 ++-----------------
>  drivers/gpu/drm/meson/meson_drv.h |  1 -
>  2 files changed, 2 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> index d3443125e661..348b5a198b9d 100644
> --- a/drivers/gpu/drm/meson/meson_drv.c
> +++ b/drivers/gpu/drm/meson/meson_drv.c
> @@ -68,15 +68,7 @@
>   * - Powering Up HDMI controller and PHY
>   */
>  
> -static void meson_fb_output_poll_changed(struct drm_device *dev)
> -{
> -	struct meson_drm *priv = dev->dev_private;
> -
> -	drm_fbdev_cma_hotplug_event(priv->fbdev);
> -}
> -
>  static const struct drm_mode_config_funcs meson_mode_config_funcs = {
> -	.output_poll_changed = meson_fb_output_poll_changed,
>  	.atomic_check        = drm_atomic_helper_check,
>  	.atomic_commit       = drm_atomic_helper_commit,
>  	.fb_create           = drm_gem_fb_create,
> @@ -282,13 +274,6 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
>  
>  	drm_mode_config_reset(drm);
>  
> -	priv->fbdev = drm_fbdev_cma_init(drm, 32,
> -					 drm->mode_config.num_connector);
> -	if (IS_ERR(priv->fbdev)) {
> -		ret = PTR_ERR(priv->fbdev);
> -		goto free_drm;
> -	}
> -
>  	drm_kms_helper_poll_init(drm);
>  
>  	platform_set_drvdata(pdev, priv);
> @@ -297,6 +282,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
>  	if (ret)
>  		goto free_drm;
>  
> +	drm_fbdev_generic_setup(drm, 32);
> +
>  	return 0;
>  
>  free_drm:
> @@ -313,11 +300,9 @@ static int meson_drv_bind(struct device *dev)
>  static void meson_drv_unbind(struct device *dev)
>  {
>  	struct drm_device *drm = dev_get_drvdata(dev);
> -	struct meson_drm *priv = drm->dev_private;
>  
>  	drm_dev_unregister(drm);
>  	drm_kms_helper_poll_fini(drm);
> -	drm_fbdev_cma_fini(priv->fbdev);
>  	drm_mode_config_cleanup(drm);
>  	drm_dev_put(drm);
>  
> diff --git a/drivers/gpu/drm/meson/meson_drv.h b/drivers/gpu/drm/meson/meson_drv.h
> index 8450d6ac8c9b..aab96260da9f 100644
> --- a/drivers/gpu/drm/meson/meson_drv.h
> +++ b/drivers/gpu/drm/meson/meson_drv.h
> @@ -33,7 +33,6 @@ struct meson_drm {
>  
>  	struct drm_device *drm;
>  	struct drm_crtc *crtc;
> -	struct drm_fbdev_cma *fbdev;
>  	struct drm_plane *primary_plane;
>  
>  	/* Components Data */
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 05/20] drm/meson: Use drm_fbdev_generic_setup()

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 1903 bytes --]

On Wed, Sep 12, 2018 at 11:48:34AM +0200, Neil Armstrong wrote:
> Hi Noralf,
> 
> On 08/09/2018 15:46, Noralf Trønnes wrote:
> > The CMA helper is already using the drm_fb_helper_generic_probe part of
> > the generic fbdev emulation. This patch makes full use of the generic
> > fbdev emulation by using its drm_client callbacks. This means that
> > drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
> > now handled by the emulation code. Additionally fbdev unregister happens
> > automatically on drm_dev_unregister().
> > 
> > The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
> > driver. This is done to highlight the fact that fbdev emulation is an
> > internal client that makes use of the driver, it is not part of the
> > driver as such. If fbdev setup fails, an error is printed, but the driver
> > succeeds probing.
> 
> I can't really ack/nack this move, on one side it's great to have a
> generic fbdev emulation instead of the old fbdev code, but on the
> other side the Amlogic platform (like allwinner, samsung, rockchip,
> ...)  still relies on an Fbdev variant of the libMali that uses
> smem_start...
> 
> I know it's dirty and fbdev based code should be legacy now, but I
> consider it like an user-space breakage...
> 
> I'll be happy if ARM provided it's Mali vendors a Fbdev libMali that
> could use FBINFO_HIDE_SMEM_START and allocate BO's from the DRM
> driver, it won't be the case and it will never be the case until the
> Lima projects finalizes.
> 
> So for me it's a no-go until Lima lands upstream in Linux *and* Mesa.

My feelings exactly. If the choice is between reducing the DRM driver
by a couple of dozens of lines or keeping the mali working, I'll pick
the latter, every single day.

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 05/20] drm/meson: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

(Cc: Daniel Vetter)


Den 12.09.2018 11.56, skrev Maxime Ripard:
> On Wed, Sep 12, 2018 at 11:48:34AM +0200, Neil Armstrong wrote:
>> Hi Noralf,
>>
>> On 08/09/2018 15:46, Noralf Trønnes wrote:
>>> The CMA helper is already using the drm_fb_helper_generic_probe part of
>>> the generic fbdev emulation. This patch makes full use of the generic
>>> fbdev emulation by using its drm_client callbacks. This means that
>>> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
>>> now handled by the emulation code. Additionally fbdev unregister happens
>>> automatically on drm_dev_unregister().
>>>
>>> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
>>> driver. This is done to highlight the fact that fbdev emulation is an
>>> internal client that makes use of the driver, it is not part of the
>>> driver as such. If fbdev setup fails, an error is printed, but the driver
>>> succeeds probing.
>> I can't really ack/nack this move, on one side it's great to have a
>> generic fbdev emulation instead of the old fbdev code, but on the
>> other side the Amlogic platform (like allwinner, samsung, rockchip,
>> ...)  still relies on an Fbdev variant of the libMali that uses
>> smem_start...
>>
>> I know it's dirty and fbdev based code should be legacy now, but I
>> consider it like an user-space breakage...
>>
>> I'll be happy if ARM provided it's Mali vendors a Fbdev libMali that
>> could use FBINFO_HIDE_SMEM_START and allocate BO's from the DRM
>> driver, it won't be the case and it will never be the case until the
>> Lima projects finalizes.
>>
>> So for me it's a no-go until Lima lands upstream in Linux *and* Mesa.
> My feelings exactly. If the choice is between reducing the DRM driver
> by a couple of dozens of lines or keeping the mali working, I'll pick
> the latter, every single day.

I don't know the reasoning for blocking smem_start other than what Daniel
wrote in these commit messages:

da6c7707caf3736c1cf968606bd97c07e79625d4
fbdev: Add FBINFO_HIDE_SMEM_START flag

   DRM drivers really, really, really don't want random userspace to
   share buffer behind it's back, bypassing the dma-buf buffer sharing
   machanism. For that reason we've ruthlessly rejected any IOCTL
   exposing the physical address of any graphics buffer.

   Unfortunately fbdev comes with that built-in. We could just set
   smem_start to 0, but that means we'd have to hand-roll our own fb_mmap
   implementation. For good reasons many drivers do that, but
   smem_start/length is still super convenient.

   Hence instead just stop the leak in the ioctl, to keep fb mmap working
   as-is. A second patch will set this flag for all drm drivers.


6be8f3bd2c78915a9f3a058a346ae93068d35c01
drm/fb: Stop leaking physical address

   For buffer sharing, use dma-buf instead. We can't set smem_start to 0
   unconditionally since that's used by the fbdev mmap default
   implementation. And we have plenty of userspace which would like to
   keep that working.

   This might break legit userspace - if it does we need to look at a
   case-by-cases basis how to handle that. Worst case I expect overrides
   for only specific drivers, since anything remotely modern should be
   using dma-buf/prime now (which is about 7 years old now for DRM
   drivers).

   This issue was uncovered because Noralf's rework to implement a
   generic fb_probe also implements it's own fb_mmap callback. Which
   means smem_start didn't have to be set anymore, which blew up some
   blob in userspace rather badly.


Noralf.

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 05/20] drm/meson: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

Den 12.09.2018 12.57, skrev Noralf Trønnes:
> (Cc: Daniel Vetter)
>

Somehow that CC was dropped somewhere after leaving email client.
Trying once more.

>
> Den 12.09.2018 11.56, skrev Maxime Ripard:
>> On Wed, Sep 12, 2018 at 11:48:34AM +0200, Neil Armstrong wrote:
>>> Hi Noralf,
>>>
>>> On 08/09/2018 15:46, Noralf Trønnes wrote:
>>>> The CMA helper is already using the drm_fb_helper_generic_probe 
>>>> part of
>>>> the generic fbdev emulation. This patch makes full use of the generic
>>>> fbdev emulation by using its drm_client callbacks. This means that
>>>> drm_mode_config_funcs->output_poll_changed and 
>>>> drm_driver->lastclose are
>>>> now handled by the emulation code. Additionally fbdev unregister 
>>>> happens
>>>> automatically on drm_dev_unregister().
>>>>
>>>> The drm_fbdev_generic_setup() call is put after drm_dev_register() 
>>>> in the
>>>> driver. This is done to highlight the fact that fbdev emulation is an
>>>> internal client that makes use of the driver, it is not part of the
>>>> driver as such. If fbdev setup fails, an error is printed, but the 
>>>> driver
>>>> succeeds probing.
>>> I can't really ack/nack this move, on one side it's great to have a
>>> generic fbdev emulation instead of the old fbdev code, but on the
>>> other side the Amlogic platform (like allwinner, samsung, rockchip,
>>> ...)  still relies on an Fbdev variant of the libMali that uses
>>> smem_start...
>>>
>>> I know it's dirty and fbdev based code should be legacy now, but I
>>> consider it like an user-space breakage...
>>>
>>> I'll be happy if ARM provided it's Mali vendors a Fbdev libMali that
>>> could use FBINFO_HIDE_SMEM_START and allocate BO's from the DRM
>>> driver, it won't be the case and it will never be the case until the
>>> Lima projects finalizes.
>>>
>>> So for me it's a no-go until Lima lands upstream in Linux *and* Mesa.
>> My feelings exactly. If the choice is between reducing the DRM driver
>> by a couple of dozens of lines or keeping the mali working, I'll pick
>> the latter, every single day.
>
> I don't know the reasoning for blocking smem_start other than what Daniel
> wrote in these commit messages:
>
> da6c7707caf3736c1cf968606bd97c07e79625d4
> fbdev: Add FBINFO_HIDE_SMEM_START flag
>
>   DRM drivers really, really, really don't want random userspace to
>   share buffer behind it's back, bypassing the dma-buf buffer sharing
>   machanism. For that reason we've ruthlessly rejected any IOCTL
>   exposing the physical address of any graphics buffer.
>
>   Unfortunately fbdev comes with that built-in. We could just set
>   smem_start to 0, but that means we'd have to hand-roll our own fb_mmap
>   implementation. For good reasons many drivers do that, but
>   smem_start/length is still super convenient.
>
>   Hence instead just stop the leak in the ioctl, to keep fb mmap working
>   as-is. A second patch will set this flag for all drm drivers.
>
>
> 6be8f3bd2c78915a9f3a058a346ae93068d35c01
> drm/fb: Stop leaking physical address
>
>   For buffer sharing, use dma-buf instead. We can't set smem_start to 0
>   unconditionally since that's used by the fbdev mmap default
>   implementation. And we have plenty of userspace which would like to
>   keep that working.
>
>   This might break legit userspace - if it does we need to look at a
>   case-by-cases basis how to handle that. Worst case I expect overrides
>   for only specific drivers, since anything remotely modern should be
>   using dma-buf/prime now (which is about 7 years old now for DRM
>   drivers).
>
>   This issue was uncovered because Noralf's rework to implement a
>   generic fb_probe also implements it's own fb_mmap callback. Which
>   means smem_start didn't have to be set anymore, which blew up some
>   blob in userspace rather badly.
>
>
> Noralf.
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 05/20] drm/meson: Use drm_fbdev_generic_setup()

[Daniel Vetter]

On Wed, Sep 12, 2018 at 01:06:07PM +0200, Noralf Trønnes wrote:
> 
> Den 12.09.2018 12.57, skrev Noralf Trønnes:
> > (Cc: Daniel Vetter)
> > 
> 
> Somehow that CC was dropped somewhere after leaving email client.
> Trying once more.

Yeah I just made myself unpopular. If you want SMEM_START, then you need
to carry a local patch now. virt_to_pfn on the vmap should work. It's
about 2 lines, including the change to drop HIDE_SMEM_START.

And if consensus is that hiding SMEM_START is not a nice idea, then I'm
sure we can reintroduce it through some slightly unpretty backdoor, even
with Noralf's generic code. So not really a good reason to reject the
cleanup I think.
-Daniel


> > Den 12.09.2018 11.56, skrev Maxime Ripard:
> > > On Wed, Sep 12, 2018 at 11:48:34AM +0200, Neil Armstrong wrote:
> > > > Hi Noralf,
> > > > 
> > > > On 08/09/2018 15:46, Noralf Trønnes wrote:
> > > > > The CMA helper is already using the
> > > > > drm_fb_helper_generic_probe part of
> > > > > the generic fbdev emulation. This patch makes full use of the generic
> > > > > fbdev emulation by using its drm_client callbacks. This means that
> > > > > drm_mode_config_funcs->output_poll_changed and
> > > > > drm_driver->lastclose are
> > > > > now handled by the emulation code. Additionally fbdev
> > > > > unregister happens
> > > > > automatically on drm_dev_unregister().
> > > > > 
> > > > > The drm_fbdev_generic_setup() call is put after
> > > > > drm_dev_register() in the
> > > > > driver. This is done to highlight the fact that fbdev emulation is an
> > > > > internal client that makes use of the driver, it is not part of the
> > > > > driver as such. If fbdev setup fails, an error is printed,
> > > > > but the driver
> > > > > succeeds probing.
> > > > I can't really ack/nack this move, on one side it's great to have a
> > > > generic fbdev emulation instead of the old fbdev code, but on the
> > > > other side the Amlogic platform (like allwinner, samsung, rockchip,
> > > > ...)  still relies on an Fbdev variant of the libMali that uses
> > > > smem_start...
> > > > 
> > > > I know it's dirty and fbdev based code should be legacy now, but I
> > > > consider it like an user-space breakage...
> > > > 
> > > > I'll be happy if ARM provided it's Mali vendors a Fbdev libMali that
> > > > could use FBINFO_HIDE_SMEM_START and allocate BO's from the DRM
> > > > driver, it won't be the case and it will never be the case until the
> > > > Lima projects finalizes.
> > > > 
> > > > So for me it's a no-go until Lima lands upstream in Linux *and* Mesa.
> > > My feelings exactly. If the choice is between reducing the DRM driver
> > > by a couple of dozens of lines or keeping the mali working, I'll pick
> > > the latter, every single day.
> > 
> > I don't know the reasoning for blocking smem_start other than what Daniel
> > wrote in these commit messages:
> > 
> > da6c7707caf3736c1cf968606bd97c07e79625d4
> > fbdev: Add FBINFO_HIDE_SMEM_START flag
> > 
> >   DRM drivers really, really, really don't want random userspace to
> >   share buffer behind it's back, bypassing the dma-buf buffer sharing
> >   machanism. For that reason we've ruthlessly rejected any IOCTL
> >   exposing the physical address of any graphics buffer.
> > 
> >   Unfortunately fbdev comes with that built-in. We could just set
> >   smem_start to 0, but that means we'd have to hand-roll our own fb_mmap
> >   implementation. For good reasons many drivers do that, but
> >   smem_start/length is still super convenient.
> > 
> >   Hence instead just stop the leak in the ioctl, to keep fb mmap working
> >   as-is. A second patch will set this flag for all drm drivers.
> > 
> > 
> > 6be8f3bd2c78915a9f3a058a346ae93068d35c01
> > drm/fb: Stop leaking physical address
> > 
> >   For buffer sharing, use dma-buf instead. We can't set smem_start to 0
> >   unconditionally since that's used by the fbdev mmap default
> >   implementation. And we have plenty of userspace which would like to
> >   keep that working.
> > 
> >   This might break legit userspace - if it does we need to look at a
> >   case-by-cases basis how to handle that. Worst case I expect overrides
> >   for only specific drivers, since anything remotely modern should be
> >   using dma-buf/prime now (which is about 7 years old now for DRM
> >   drivers).
> > 
> >   This issue was uncovered because Noralf's rework to implement a
> >   generic fb_probe also implements it's own fb_mmap callback. Which
> >   means smem_start didn't have to be set anymore, which blew up some
> >   blob in userspace rather badly.
> > 
> > 
> > Noralf.
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> 

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

Re: [PATCH 05/20] drm/meson: Use drm_fbdev_generic_setup()

[Neil Armstrong]

Hi Daniel,

On 13/09/2018 15:21, Daniel Vetter wrote:
> On Wed, Sep 12, 2018 at 01:06:07PM +0200, Noralf Trønnes wrote:
>>
>> Den 12.09.2018 12.57, skrev Noralf Trønnes:
>>> (Cc: Daniel Vetter)
>>>
>>
>> Somehow that CC was dropped somewhere after leaving email client.
>> Trying once more.
> 
> Yeah I just made myself unpopular. If you want SMEM_START, then you need
> to carry a local patch now. virt_to_pfn on the vmap should work. It's
> about 2 lines, including the change to drop HIDE_SMEM_START.

Would it be totally unacceptable to add such 2 line :
- enabled by a specific config depending on EXPERT and narrowed to specific platforms
- printing a big fat pr_warning_once when used
- with a big fat comment specifying when this code will be dropped and why we should not activate it

Neil
> 
> And if consensus is that hiding SMEM_START is not a nice idea, then I'm
> sure we can reintroduce it through some slightly unpretty backdoor, even
> with Noralf's generic code. So not really a good reason to reject the
> cleanup I think.
> -Daniel
> 
> 
>>> Den 12.09.2018 11.56, skrev Maxime Ripard:
>>>> On Wed, Sep 12, 2018 at 11:48:34AM +0200, Neil Armstrong wrote:
>>>>> Hi Noralf,
>>>>>
>>>>> On 08/09/2018 15:46, Noralf Trønnes wrote:
>>>>>> The CMA helper is already using the
>>>>>> drm_fb_helper_generic_probe part of
>>>>>> the generic fbdev emulation. This patch makes full use of the generic
>>>>>> fbdev emulation by using its drm_client callbacks. This means that
>>>>>> drm_mode_config_funcs->output_poll_changed and
>>>>>> drm_driver->lastclose are
>>>>>> now handled by the emulation code. Additionally fbdev
>>>>>> unregister happens
>>>>>> automatically on drm_dev_unregister().
>>>>>>
>>>>>> The drm_fbdev_generic_setup() call is put after
>>>>>> drm_dev_register() in the
>>>>>> driver. This is done to highlight the fact that fbdev emulation is an
>>>>>> internal client that makes use of the driver, it is not part of the
>>>>>> driver as such. If fbdev setup fails, an error is printed,
>>>>>> but the driver
>>>>>> succeeds probing.
>>>>> I can't really ack/nack this move, on one side it's great to have a
>>>>> generic fbdev emulation instead of the old fbdev code, but on the
>>>>> other side the Amlogic platform (like allwinner, samsung, rockchip,
>>>>> ...)  still relies on an Fbdev variant of the libMali that uses
>>>>> smem_start...
>>>>>
>>>>> I know it's dirty and fbdev based code should be legacy now, but I
>>>>> consider it like an user-space breakage...
>>>>>
>>>>> I'll be happy if ARM provided it's Mali vendors a Fbdev libMali that
>>>>> could use FBINFO_HIDE_SMEM_START and allocate BO's from the DRM
>>>>> driver, it won't be the case and it will never be the case until the
>>>>> Lima projects finalizes.
>>>>>
>>>>> So for me it's a no-go until Lima lands upstream in Linux *and* Mesa.
>>>> My feelings exactly. If the choice is between reducing the DRM driver
>>>> by a couple of dozens of lines or keeping the mali working, I'll pick
>>>> the latter, every single day.
>>>
>>> I don't know the reasoning for blocking smem_start other than what Daniel
>>> wrote in these commit messages:
>>>
>>> da6c7707caf3736c1cf968606bd97c07e79625d4
>>> fbdev: Add FBINFO_HIDE_SMEM_START flag
>>>
>>>   DRM drivers really, really, really don't want random userspace to
>>>   share buffer behind it's back, bypassing the dma-buf buffer sharing
>>>   machanism. For that reason we've ruthlessly rejected any IOCTL
>>>   exposing the physical address of any graphics buffer.
>>>
>>>   Unfortunately fbdev comes with that built-in. We could just set
>>>   smem_start to 0, but that means we'd have to hand-roll our own fb_mmap
>>>   implementation. For good reasons many drivers do that, but
>>>   smem_start/length is still super convenient.
>>>
>>>   Hence instead just stop the leak in the ioctl, to keep fb mmap working
>>>   as-is. A second patch will set this flag for all drm drivers.
>>>
>>>
>>> 6be8f3bd2c78915a9f3a058a346ae93068d35c01
>>> drm/fb: Stop leaking physical address
>>>
>>>   For buffer sharing, use dma-buf instead. We can't set smem_start to 0
>>>   unconditionally since that's used by the fbdev mmap default
>>>   implementation. And we have plenty of userspace which would like to
>>>   keep that working.
>>>
>>>   This might break legit userspace - if it does we need to look at a
>>>   case-by-cases basis how to handle that. Worst case I expect overrides
>>>   for only specific drivers, since anything remotely modern should be
>>>   using dma-buf/prime now (which is about 7 years old now for DRM
>>>   drivers).
>>>
>>>   This issue was uncovered because Noralf's rework to implement a
>>>   generic fb_probe also implements it's own fb_mmap callback. Which
>>>   means smem_start didn't have to be set anymore, which blew up some
>>>   blob in userspace rather badly.
>>>
>>>
>>> Noralf.
>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>>
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 05/20] drm/meson: Use drm_fbdev_generic_setup()

[Daniel Vetter]

On Thu, Sep 13, 2018 at 04:26:53PM +0200, Neil Armstrong wrote:
> Hi Daniel,
> 
> On 13/09/2018 15:21, Daniel Vetter wrote:
> > On Wed, Sep 12, 2018 at 01:06:07PM +0200, Noralf Trønnes wrote:
> >>
> >> Den 12.09.2018 12.57, skrev Noralf Trønnes:
> >>> (Cc: Daniel Vetter)
> >>>
> >>
> >> Somehow that CC was dropped somewhere after leaving email client.
> >> Trying once more.
> > 
> > Yeah I just made myself unpopular. If you want SMEM_START, then you need
> > to carry a local patch now. virt_to_pfn on the vmap should work. It's
> > about 2 lines, including the change to drop HIDE_SMEM_START.
> 
> Would it be totally unacceptable to add such 2 line :
> - enabled by a specific config depending on EXPERT and narrowed to specific platforms
> - printing a big fat pr_warning_once when used
> - with a big fat comment specifying when this code will be dropped and why we should not activate it

module_param_debug auto-taints your kernel, I'd just go with that. Plus
CONFIG_EXPERT (or CONFIG_BROKEN).

But yes, I think that's something I'll happily ack. Must be acked by Dave
(and perhaps a few others too), since defacto this means we now suddenly
care about closed source blobs. I'd say get someone from amd and a few of
the driver maintainers on board with this (nouveau, Eric, Rob, Lucas,
...), to make sure it really represents community consensus.

Cheers, Daniel

> 
> Neil
> > 
> > And if consensus is that hiding SMEM_START is not a nice idea, then I'm
> > sure we can reintroduce it through some slightly unpretty backdoor, even
> > with Noralf's generic code. So not really a good reason to reject the
> > cleanup I think.
> > -Daniel
> > 
> > 
> >>> Den 12.09.2018 11.56, skrev Maxime Ripard:
> >>>> On Wed, Sep 12, 2018 at 11:48:34AM +0200, Neil Armstrong wrote:
> >>>>> Hi Noralf,
> >>>>>
> >>>>> On 08/09/2018 15:46, Noralf Trønnes wrote:
> >>>>>> The CMA helper is already using the
> >>>>>> drm_fb_helper_generic_probe part of
> >>>>>> the generic fbdev emulation. This patch makes full use of the generic
> >>>>>> fbdev emulation by using its drm_client callbacks. This means that
> >>>>>> drm_mode_config_funcs->output_poll_changed and
> >>>>>> drm_driver->lastclose are
> >>>>>> now handled by the emulation code. Additionally fbdev
> >>>>>> unregister happens
> >>>>>> automatically on drm_dev_unregister().
> >>>>>>
> >>>>>> The drm_fbdev_generic_setup() call is put after
> >>>>>> drm_dev_register() in the
> >>>>>> driver. This is done to highlight the fact that fbdev emulation is an
> >>>>>> internal client that makes use of the driver, it is not part of the
> >>>>>> driver as such. If fbdev setup fails, an error is printed,
> >>>>>> but the driver
> >>>>>> succeeds probing.
> >>>>> I can't really ack/nack this move, on one side it's great to have a
> >>>>> generic fbdev emulation instead of the old fbdev code, but on the
> >>>>> other side the Amlogic platform (like allwinner, samsung, rockchip,
> >>>>> ...)  still relies on an Fbdev variant of the libMali that uses
> >>>>> smem_start...
> >>>>>
> >>>>> I know it's dirty and fbdev based code should be legacy now, but I
> >>>>> consider it like an user-space breakage...
> >>>>>
> >>>>> I'll be happy if ARM provided it's Mali vendors a Fbdev libMali that
> >>>>> could use FBINFO_HIDE_SMEM_START and allocate BO's from the DRM
> >>>>> driver, it won't be the case and it will never be the case until the
> >>>>> Lima projects finalizes.
> >>>>>
> >>>>> So for me it's a no-go until Lima lands upstream in Linux *and* Mesa.
> >>>> My feelings exactly. If the choice is between reducing the DRM driver
> >>>> by a couple of dozens of lines or keeping the mali working, I'll pick
> >>>> the latter, every single day.
> >>>
> >>> I don't know the reasoning for blocking smem_start other than what Daniel
> >>> wrote in these commit messages:
> >>>
> >>> da6c7707caf3736c1cf968606bd97c07e79625d4
> >>> fbdev: Add FBINFO_HIDE_SMEM_START flag
> >>>
> >>>   DRM drivers really, really, really don't want random userspace to
> >>>   share buffer behind it's back, bypassing the dma-buf buffer sharing
> >>>   machanism. For that reason we've ruthlessly rejected any IOCTL
> >>>   exposing the physical address of any graphics buffer.
> >>>
> >>>   Unfortunately fbdev comes with that built-in. We could just set
> >>>   smem_start to 0, but that means we'd have to hand-roll our own fb_mmap
> >>>   implementation. For good reasons many drivers do that, but
> >>>   smem_start/length is still super convenient.
> >>>
> >>>   Hence instead just stop the leak in the ioctl, to keep fb mmap working
> >>>   as-is. A second patch will set this flag for all drm drivers.
> >>>
> >>>
> >>> 6be8f3bd2c78915a9f3a058a346ae93068d35c01
> >>> drm/fb: Stop leaking physical address
> >>>
> >>>   For buffer sharing, use dma-buf instead. We can't set smem_start to 0
> >>>   unconditionally since that's used by the fbdev mmap default
> >>>   implementation. And we have plenty of userspace which would like to
> >>>   keep that working.
> >>>
> >>>   This might break legit userspace - if it does we need to look at a
> >>>   case-by-cases basis how to handle that. Worst case I expect overrides
> >>>   for only specific drivers, since anything remotely modern should be
> >>>   using dma-buf/prime now (which is about 7 years old now for DRM
> >>>   drivers).
> >>>
> >>>   This issue was uncovered because Noralf's rework to implement a
> >>>   generic fb_probe also implements it's own fb_mmap callback. Which
> >>>   means smem_start didn't have to be set anymore, which blew up some
> >>>   blob in userspace rather badly.
> >>>
> >>>
> >>> Noralf.
> >>>
> >>> _______________________________________________
> >>> dri-devel mailing list
> >>> dri-devel@lists.freedesktop.org
> >>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >>>
> >>
> > 
> 

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

Re: [PATCH 05/20] drm/meson: Use drm_fbdev_generic_setup()

[Neil Armstrong]

Hi Daniel,

On 13/09/2018 16:55, Daniel Vetter wrote:
> On Thu, Sep 13, 2018 at 04:26:53PM +0200, Neil Armstrong wrote:
>> Hi Daniel,
>>
>> On 13/09/2018 15:21, Daniel Vetter wrote:
>>> On Wed, Sep 12, 2018 at 01:06:07PM +0200, Noralf Trønnes wrote:
>>>>
>>>> Den 12.09.2018 12.57, skrev Noralf Trønnes:
>>>>> (Cc: Daniel Vetter)
>>>>>
>>>>
>>>> Somehow that CC was dropped somewhere after leaving email client.
>>>> Trying once more.
>>>
>>> Yeah I just made myself unpopular. If you want SMEM_START, then you need
>>> to carry a local patch now. virt_to_pfn on the vmap should work. It's
>>> about 2 lines, including the change to drop HIDE_SMEM_START.
>>
>> Would it be totally unacceptable to add such 2 line :
>> - enabled by a specific config depending on EXPERT and narrowed to specific platforms
>> - printing a big fat pr_warning_once when used
>> - with a big fat comment specifying when this code will be dropped and why we should not activate it
> 
> module_param_debug auto-taints your kernel, I'd just go with that. Plus
> CONFIG_EXPERT (or CONFIG_BROKEN).

OK, I think you mean module_param_unsafe, but I see the point.

> 
> But yes, I think that's something I'll happily ack. Must be acked by Dave
> (and perhaps a few others too), since defacto this means we now suddenly
> care about closed source blobs. I'd say get someone from amd and a few of
> the driver maintainers on board with this (nouveau, Eric, Rob, Lucas,
> ...), to make sure it really represents community consensus.

I'll drop something, but I'm afraid a kernel won't have this hack, shouldn't this serie be delayed for a release ?

@Noaralf, do you have a branch somewhere I can base a work on ?

Neil

> 
> Cheers, Daniel
> 
>>
>> Neil
>>>
>>> And if consensus is that hiding SMEM_START is not a nice idea, then I'm
>>> sure we can reintroduce it through some slightly unpretty backdoor, even
>>> with Noralf's generic code. So not really a good reason to reject the
>>> cleanup I think.
>>> -Daniel
>>>
>>>
>>>>> Den 12.09.2018 11.56, skrev Maxime Ripard:
>>>>>> On Wed, Sep 12, 2018 at 11:48:34AM +0200, Neil Armstrong wrote:
>>>>>>> Hi Noralf,
>>>>>>>
>>>>>>> On 08/09/2018 15:46, Noralf Trønnes wrote:
>>>>>>>> The CMA helper is already using the
>>>>>>>> drm_fb_helper_generic_probe part of
>>>>>>>> the generic fbdev emulation. This patch makes full use of the generic
>>>>>>>> fbdev emulation by using its drm_client callbacks. This means that
>>>>>>>> drm_mode_config_funcs->output_poll_changed and
>>>>>>>> drm_driver->lastclose are
>>>>>>>> now handled by the emulation code. Additionally fbdev
>>>>>>>> unregister happens
>>>>>>>> automatically on drm_dev_unregister().
>>>>>>>>
>>>>>>>> The drm_fbdev_generic_setup() call is put after
>>>>>>>> drm_dev_register() in the
>>>>>>>> driver. This is done to highlight the fact that fbdev emulation is an
>>>>>>>> internal client that makes use of the driver, it is not part of the
>>>>>>>> driver as such. If fbdev setup fails, an error is printed,
>>>>>>>> but the driver
>>>>>>>> succeeds probing.
>>>>>>> I can't really ack/nack this move, on one side it's great to have a
>>>>>>> generic fbdev emulation instead of the old fbdev code, but on the
>>>>>>> other side the Amlogic platform (like allwinner, samsung, rockchip,
>>>>>>> ...)  still relies on an Fbdev variant of the libMali that uses
>>>>>>> smem_start...
>>>>>>>
>>>>>>> I know it's dirty and fbdev based code should be legacy now, but I
>>>>>>> consider it like an user-space breakage...
>>>>>>>
>>>>>>> I'll be happy if ARM provided it's Mali vendors a Fbdev libMali that
>>>>>>> could use FBINFO_HIDE_SMEM_START and allocate BO's from the DRM
>>>>>>> driver, it won't be the case and it will never be the case until the
>>>>>>> Lima projects finalizes.
>>>>>>>
>>>>>>> So for me it's a no-go until Lima lands upstream in Linux *and* Mesa.
>>>>>> My feelings exactly. If the choice is between reducing the DRM driver
>>>>>> by a couple of dozens of lines or keeping the mali working, I'll pick
>>>>>> the latter, every single day.
>>>>>
>>>>> I don't know the reasoning for blocking smem_start other than what Daniel
>>>>> wrote in these commit messages:
>>>>>
>>>>> da6c7707caf3736c1cf968606bd97c07e79625d4
>>>>> fbdev: Add FBINFO_HIDE_SMEM_START flag
>>>>>
>>>>>   DRM drivers really, really, really don't want random userspace to
>>>>>   share buffer behind it's back, bypassing the dma-buf buffer sharing
>>>>>   machanism. For that reason we've ruthlessly rejected any IOCTL
>>>>>   exposing the physical address of any graphics buffer.
>>>>>
>>>>>   Unfortunately fbdev comes with that built-in. We could just set
>>>>>   smem_start to 0, but that means we'd have to hand-roll our own fb_mmap
>>>>>   implementation. For good reasons many drivers do that, but
>>>>>   smem_start/length is still super convenient.
>>>>>
>>>>>   Hence instead just stop the leak in the ioctl, to keep fb mmap working
>>>>>   as-is. A second patch will set this flag for all drm drivers.
>>>>>
>>>>>
>>>>> 6be8f3bd2c78915a9f3a058a346ae93068d35c01
>>>>> drm/fb: Stop leaking physical address
>>>>>
>>>>>   For buffer sharing, use dma-buf instead. We can't set smem_start to 0
>>>>>   unconditionally since that's used by the fbdev mmap default
>>>>>   implementation. And we have plenty of userspace which would like to
>>>>>   keep that working.
>>>>>
>>>>>   This might break legit userspace - if it does we need to look at a
>>>>>   case-by-cases basis how to handle that. Worst case I expect overrides
>>>>>   for only specific drivers, since anything remotely modern should be
>>>>>   using dma-buf/prime now (which is about 7 years old now for DRM
>>>>>   drivers).
>>>>>
>>>>>   This issue was uncovered because Noralf's rework to implement a
>>>>>   generic fb_probe also implements it's own fb_mmap callback. Which
>>>>>   means smem_start didn't have to be set anymore, which blew up some
>>>>>   blob in userspace rather badly.
>>>>>
>>>>>
>>>>> Noralf.
>>>>>
>>>>> _______________________________________________
>>>>> dri-devel mailing list
>>>>> dri-devel@lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>>
>>>>
>>>
>>
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 05/20] drm/meson: Use drm_fbdev_generic_setup()

[Daniel Vetter]

On Fri, Sep 14, 2018 at 10:23 AM, Neil Armstrong
<narmstrong@baylibre.com> wrote:
> Hi Daniel,
>
> On 13/09/2018 16:55, Daniel Vetter wrote:
>> On Thu, Sep 13, 2018 at 04:26:53PM +0200, Neil Armstrong wrote:
>>> Hi Daniel,
>>>
>>> On 13/09/2018 15:21, Daniel Vetter wrote:
>>>> On Wed, Sep 12, 2018 at 01:06:07PM +0200, Noralf Trønnes wrote:
>>>>>
>>>>> Den 12.09.2018 12.57, skrev Noralf Trønnes:
>>>>>> (Cc: Daniel Vetter)
>>>>>>
>>>>>
>>>>> Somehow that CC was dropped somewhere after leaving email client.
>>>>> Trying once more.
>>>>
>>>> Yeah I just made myself unpopular. If you want SMEM_START, then you need
>>>> to carry a local patch now. virt_to_pfn on the vmap should work. It's
>>>> about 2 lines, including the change to drop HIDE_SMEM_START.
>>>
>>> Would it be totally unacceptable to add such 2 line :
>>> - enabled by a specific config depending on EXPERT and narrowed to specific platforms
>>> - printing a big fat pr_warning_once when used
>>> - with a big fat comment specifying when this code will be dropped and why we should not activate it
>>
>> module_param_debug auto-taints your kernel, I'd just go with that. Plus
>> CONFIG_EXPERT (or CONFIG_BROKEN).
>
> OK, I think you mean module_param_unsafe, but I see the point.
>
>>
>> But yes, I think that's something I'll happily ack. Must be acked by Dave
>> (and perhaps a few others too), since defacto this means we now suddenly
>> care about closed source blobs. I'd say get someone from amd and a few of
>> the driver maintainers on board with this (nouveau, Eric, Rob, Lucas,
>> ...), to make sure it really represents community consensus.
>
> I'll drop something, but I'm afraid a kernel won't have this hack, shouldn't this serie be delayed for a release ?

smem_start is gone already, for everyone. If you want to delay this,
then either you need to revert my commits, or get your hack in
quickly.
-Daniel

> @Noaralf, do you have a branch somewhere I can base a work on ?
>
> Neil
>
>>
>> Cheers, Daniel
>>
>>>
>>> Neil
>>>>
>>>> And if consensus is that hiding SMEM_START is not a nice idea, then I'm
>>>> sure we can reintroduce it through some slightly unpretty backdoor, even
>>>> with Noralf's generic code. So not really a good reason to reject the
>>>> cleanup I think.
>>>> -Daniel
>>>>
>>>>
>>>>>> Den 12.09.2018 11.56, skrev Maxime Ripard:
>>>>>>> On Wed, Sep 12, 2018 at 11:48:34AM +0200, Neil Armstrong wrote:
>>>>>>>> Hi Noralf,
>>>>>>>>
>>>>>>>> On 08/09/2018 15:46, Noralf Trønnes wrote:
>>>>>>>>> The CMA helper is already using the
>>>>>>>>> drm_fb_helper_generic_probe part of
>>>>>>>>> the generic fbdev emulation. This patch makes full use of the generic
>>>>>>>>> fbdev emulation by using its drm_client callbacks. This means that
>>>>>>>>> drm_mode_config_funcs->output_poll_changed and
>>>>>>>>> drm_driver->lastclose are
>>>>>>>>> now handled by the emulation code. Additionally fbdev
>>>>>>>>> unregister happens
>>>>>>>>> automatically on drm_dev_unregister().
>>>>>>>>>
>>>>>>>>> The drm_fbdev_generic_setup() call is put after
>>>>>>>>> drm_dev_register() in the
>>>>>>>>> driver. This is done to highlight the fact that fbdev emulation is an
>>>>>>>>> internal client that makes use of the driver, it is not part of the
>>>>>>>>> driver as such. If fbdev setup fails, an error is printed,
>>>>>>>>> but the driver
>>>>>>>>> succeeds probing.
>>>>>>>> I can't really ack/nack this move, on one side it's great to have a
>>>>>>>> generic fbdev emulation instead of the old fbdev code, but on the
>>>>>>>> other side the Amlogic platform (like allwinner, samsung, rockchip,
>>>>>>>> ...)  still relies on an Fbdev variant of the libMali that uses
>>>>>>>> smem_start...
>>>>>>>>
>>>>>>>> I know it's dirty and fbdev based code should be legacy now, but I
>>>>>>>> consider it like an user-space breakage...
>>>>>>>>
>>>>>>>> I'll be happy if ARM provided it's Mali vendors a Fbdev libMali that
>>>>>>>> could use FBINFO_HIDE_SMEM_START and allocate BO's from the DRM
>>>>>>>> driver, it won't be the case and it will never be the case until the
>>>>>>>> Lima projects finalizes.
>>>>>>>>
>>>>>>>> So for me it's a no-go until Lima lands upstream in Linux *and* Mesa.
>>>>>>> My feelings exactly. If the choice is between reducing the DRM driver
>>>>>>> by a couple of dozens of lines or keeping the mali working, I'll pick
>>>>>>> the latter, every single day.
>>>>>>
>>>>>> I don't know the reasoning for blocking smem_start other than what Daniel
>>>>>> wrote in these commit messages:
>>>>>>
>>>>>> da6c7707caf3736c1cf968606bd97c07e79625d4
>>>>>> fbdev: Add FBINFO_HIDE_SMEM_START flag
>>>>>>
>>>>>>   DRM drivers really, really, really don't want random userspace to
>>>>>>   share buffer behind it's back, bypassing the dma-buf buffer sharing
>>>>>>   machanism. For that reason we've ruthlessly rejected any IOCTL
>>>>>>   exposing the physical address of any graphics buffer.
>>>>>>
>>>>>>   Unfortunately fbdev comes with that built-in. We could just set
>>>>>>   smem_start to 0, but that means we'd have to hand-roll our own fb_mmap
>>>>>>   implementation. For good reasons many drivers do that, but
>>>>>>   smem_start/length is still super convenient.
>>>>>>
>>>>>>   Hence instead just stop the leak in the ioctl, to keep fb mmap working
>>>>>>   as-is. A second patch will set this flag for all drm drivers.
>>>>>>
>>>>>>
>>>>>> 6be8f3bd2c78915a9f3a058a346ae93068d35c01
>>>>>> drm/fb: Stop leaking physical address
>>>>>>
>>>>>>   For buffer sharing, use dma-buf instead. We can't set smem_start to 0
>>>>>>   unconditionally since that's used by the fbdev mmap default
>>>>>>   implementation. And we have plenty of userspace which would like to
>>>>>>   keep that working.
>>>>>>
>>>>>>   This might break legit userspace - if it does we need to look at a
>>>>>>   case-by-cases basis how to handle that. Worst case I expect overrides
>>>>>>   for only specific drivers, since anything remotely modern should be
>>>>>>   using dma-buf/prime now (which is about 7 years old now for DRM
>>>>>>   drivers).
>>>>>>
>>>>>>   This issue was uncovered because Noralf's rework to implement a
>>>>>>   generic fb_probe also implements it's own fb_mmap callback. Which
>>>>>>   means smem_start didn't have to be set anymore, which blew up some
>>>>>>   blob in userspace rather badly.
>>>>>>
>>>>>>
>>>>>> Noralf.
>>>>>>
>>>>>> _______________________________________________
>>>>>> dri-devel mailing list
>>>>>> dri-devel@lists.freedesktop.org
>>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>>>
>>>>>
>>>>
>>>
>>
>



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 05/20] drm/meson: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

Den 14.09.2018 10.23, skrev Neil Armstrong:
> Hi Daniel,
>
> On 13/09/2018 16:55, Daniel Vetter wrote:
>> On Thu, Sep 13, 2018 at 04:26:53PM +0200, Neil Armstrong wrote:
>>> Hi Daniel,
>>>
>>> On 13/09/2018 15:21, Daniel Vetter wrote:
>>>> On Wed, Sep 12, 2018 at 01:06:07PM +0200, Noralf Trønnes wrote:
>>>>> Den 12.09.2018 12.57, skrev Noralf Trønnes:
>>>>>> (Cc: Daniel Vetter)
>>>>>>
>>>>> Somehow that CC was dropped somewhere after leaving email client.
>>>>> Trying once more.
>>>> Yeah I just made myself unpopular. If you want SMEM_START, then you need
>>>> to carry a local patch now. virt_to_pfn on the vmap should work. It's
>>>> about 2 lines, including the change to drop HIDE_SMEM_START.
>>> Would it be totally unacceptable to add such 2 line :
>>> - enabled by a specific config depending on EXPERT and narrowed to specific platforms
>>> - printing a big fat pr_warning_once when used
>>> - with a big fat comment specifying when this code will be dropped and why we should not activate it
>> module_param_debug auto-taints your kernel, I'd just go with that. Plus
>> CONFIG_EXPERT (or CONFIG_BROKEN).
> OK, I think you mean module_param_unsafe, but I see the point.
>
>> But yes, I think that's something I'll happily ack. Must be acked by Dave
>> (and perhaps a few others too), since defacto this means we now suddenly
>> care about closed source blobs. I'd say get someone from amd and a few of
>> the driver maintainers on board with this (nouveau, Eric, Rob, Lucas,
>> ...), to make sure it really represents community consensus.
> I'll drop something, but I'm afraid a kernel won't have this hack, shouldn't this serie be delayed for a release ?

It's not this series that drops smem_start support.
It happened in commit 894a677f4b3e:
drm/cma-helper: Use the generic fbdev emulation

This series only deals with the fb_helper callbacks.

> @Noaralf, do you have a branch somewhere I can base a work on ?

I haven't got a git repo, but you can apply the patches from patchwork:

[01/20] drm/fb-helper: Improve error reporting in setup
curl https://patchwork.freedesktop.org/patch/247860/mbox/ | git am

[05/20] drm/meson: Use drm_fbdev_generic_setup()
curl https://patchwork.freedesktop.org/patch/247868/mbox/ | git am

Noralf.

> Neil
>
>> Cheers, Daniel
>>
>>> Neil
>>>> And if consensus is that hiding SMEM_START is not a nice idea, then I'm
>>>> sure we can reintroduce it through some slightly unpretty backdoor, even
>>>> with Noralf's generic code. So not really a good reason to reject the
>>>> cleanup I think.
>>>> -Daniel
>>>>
>>>>
>>>>>> Den 12.09.2018 11.56, skrev Maxime Ripard:
>>>>>>> On Wed, Sep 12, 2018 at 11:48:34AM +0200, Neil Armstrong wrote:
>>>>>>>> Hi Noralf,
>>>>>>>>
>>>>>>>> On 08/09/2018 15:46, Noralf Trønnes wrote:
>>>>>>>>> The CMA helper is already using the
>>>>>>>>> drm_fb_helper_generic_probe part of
>>>>>>>>> the generic fbdev emulation. This patch makes full use of the generic
>>>>>>>>> fbdev emulation by using its drm_client callbacks. This means that
>>>>>>>>> drm_mode_config_funcs->output_poll_changed and
>>>>>>>>> drm_driver->lastclose are
>>>>>>>>> now handled by the emulation code. Additionally fbdev
>>>>>>>>> unregister happens
>>>>>>>>> automatically on drm_dev_unregister().
>>>>>>>>>
>>>>>>>>> The drm_fbdev_generic_setup() call is put after
>>>>>>>>> drm_dev_register() in the
>>>>>>>>> driver. This is done to highlight the fact that fbdev emulation is an
>>>>>>>>> internal client that makes use of the driver, it is not part of the
>>>>>>>>> driver as such. If fbdev setup fails, an error is printed,
>>>>>>>>> but the driver
>>>>>>>>> succeeds probing.
>>>>>>>> I can't really ack/nack this move, on one side it's great to have a
>>>>>>>> generic fbdev emulation instead of the old fbdev code, but on the
>>>>>>>> other side the Amlogic platform (like allwinner, samsung, rockchip,
>>>>>>>> ...)  still relies on an Fbdev variant of the libMali that uses
>>>>>>>> smem_start...
>>>>>>>>
>>>>>>>> I know it's dirty and fbdev based code should be legacy now, but I
>>>>>>>> consider it like an user-space breakage...
>>>>>>>>
>>>>>>>> I'll be happy if ARM provided it's Mali vendors a Fbdev libMali that
>>>>>>>> could use FBINFO_HIDE_SMEM_START and allocate BO's from the DRM
>>>>>>>> driver, it won't be the case and it will never be the case until the
>>>>>>>> Lima projects finalizes.
>>>>>>>>
>>>>>>>> So for me it's a no-go until Lima lands upstream in Linux *and* Mesa.
>>>>>>> My feelings exactly. If the choice is between reducing the DRM driver
>>>>>>> by a couple of dozens of lines or keeping the mali working, I'll pick
>>>>>>> the latter, every single day.
>>>>>> I don't know the reasoning for blocking smem_start other than what Daniel
>>>>>> wrote in these commit messages:
>>>>>>
>>>>>> da6c7707caf3736c1cf968606bd97c07e79625d4
>>>>>> fbdev: Add FBINFO_HIDE_SMEM_START flag
>>>>>>
>>>>>>    DRM drivers really, really, really don't want random userspace to
>>>>>>    share buffer behind it's back, bypassing the dma-buf buffer sharing
>>>>>>    machanism. For that reason we've ruthlessly rejected any IOCTL
>>>>>>    exposing the physical address of any graphics buffer.
>>>>>>
>>>>>>    Unfortunately fbdev comes with that built-in. We could just set
>>>>>>    smem_start to 0, but that means we'd have to hand-roll our own fb_mmap
>>>>>>    implementation. For good reasons many drivers do that, but
>>>>>>    smem_start/length is still super convenient.
>>>>>>
>>>>>>    Hence instead just stop the leak in the ioctl, to keep fb mmap working
>>>>>>    as-is. A second patch will set this flag for all drm drivers.
>>>>>>
>>>>>>
>>>>>> 6be8f3bd2c78915a9f3a058a346ae93068d35c01
>>>>>> drm/fb: Stop leaking physical address
>>>>>>
>>>>>>    For buffer sharing, use dma-buf instead. We can't set smem_start to 0
>>>>>>    unconditionally since that's used by the fbdev mmap default
>>>>>>    implementation. And we have plenty of userspace which would like to
>>>>>>    keep that working.
>>>>>>
>>>>>>    This might break legit userspace - if it does we need to look at a
>>>>>>    case-by-cases basis how to handle that. Worst case I expect overrides
>>>>>>    for only specific drivers, since anything remotely modern should be
>>>>>>    using dma-buf/prime now (which is about 7 years old now for DRM
>>>>>>    drivers).
>>>>>>
>>>>>>    This issue was uncovered because Noralf's rework to implement a
>>>>>>    generic fb_probe also implements it's own fb_mmap callback. Which
>>>>>>    means smem_start didn't have to be set anymore, which blew up some
>>>>>>    blob in userspace rather badly.
>>>>>>
>>>>>>
>>>>>> Noralf.
>>>>>>
>>>>>> _______________________________________________
>>>>>> dri-devel mailing list
>>>>>> dri-devel@lists.freedesktop.org
>>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>>>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 05/20] drm/meson: Use drm_fbdev_generic_setup()

[Neil Armstrong]

Hi Noralf, Daniel,

On 14/09/2018 18:33, Noralf Trønnes wrote:
> 
> Den 14.09.2018 10.23, skrev Neil Armstrong:
>> Hi Daniel,
>>
>> On 13/09/2018 16:55, Daniel Vetter wrote:
>>> On Thu, Sep 13, 2018 at 04:26:53PM +0200, Neil Armstrong wrote:
>>>> Hi Daniel,
>>>>
>>>> On 13/09/2018 15:21, Daniel Vetter wrote:
>>>>> On Wed, Sep 12, 2018 at 01:06:07PM +0200, Noralf Trønnes wrote:
>>>>>> Den 12.09.2018 12.57, skrev Noralf Trønnes:
>>>>>>> (Cc: Daniel Vetter)
>>>>>>>
>>>>>> Somehow that CC was dropped somewhere after leaving email client.
>>>>>> Trying once more.
>>>>> Yeah I just made myself unpopular. If you want SMEM_START, then you need
>>>>> to carry a local patch now. virt_to_pfn on the vmap should work. It's
>>>>> about 2 lines, including the change to drop HIDE_SMEM_START.
>>>> Would it be totally unacceptable to add such 2 line :
>>>> - enabled by a specific config depending on EXPERT and narrowed to specific platforms
>>>> - printing a big fat pr_warning_once when used
>>>> - with a big fat comment specifying when this code will be dropped and why we should not activate it
>>> module_param_debug auto-taints your kernel, I'd just go with that. Plus
>>> CONFIG_EXPERT (or CONFIG_BROKEN).
>> OK, I think you mean module_param_unsafe, but I see the point.
>>
>>> But yes, I think that's something I'll happily ack. Must be acked by Dave
>>> (and perhaps a few others too), since defacto this means we now suddenly
>>> care about closed source blobs. I'd say get someone from amd and a few of
>>> the driver maintainers on board with this (nouveau, Eric, Rob, Lucas,
>>> ...), to make sure it really represents community consensus.
>> I'll drop something, but I'm afraid a kernel won't have this hack, shouldn't this serie be delayed for a release ?
> 
> It's not this series that drops smem_start support.
> It happened in commit 894a677f4b3e:
> drm/cma-helper: Use the generic fbdev emulation
> 
> This series only deals with the fb_helper callbacks.
> 
>> @Noaralf, do you have a branch somewhere I can base a work on ?
> 
> I haven't got a git repo, but you can apply the patches from patchwork:
> 
> [01/20] drm/fb-helper: Improve error reporting in setup
> curl https://patchwork.freedesktop.org/patch/247860/mbox/ | git am
> 
> [05/20] drm/meson: Use drm_fbdev_generic_setup()
> curl https://patchwork.freedesktop.org/patch/247868/mbox/ | git am

Thanks,

I'll try to drop something, but not immediately.

Neil

> 
> Noralf.
> 
>> Neil
>>
>>> Cheers, Daniel
>>>
>>>> Neil
>>>>> And if consensus is that hiding SMEM_START is not a nice idea, then I'm
>>>>> sure we can reintroduce it through some slightly unpretty backdoor, even
>>>>> with Noralf's generic code. So not really a good reason to reject the
>>>>> cleanup I think.
>>>>> -Daniel
>>>>>
>>>>>
>>>>>>> Den 12.09.2018 11.56, skrev Maxime Ripard:
>>>>>>>> On Wed, Sep 12, 2018 at 11:48:34AM +0200, Neil Armstrong wrote:
>>>>>>>>> Hi Noralf,
>>>>>>>>>
>>>>>>>>> On 08/09/2018 15:46, Noralf Trønnes wrote:
>>>>>>>>>> The CMA helper is already using the
>>>>>>>>>> drm_fb_helper_generic_probe part of
>>>>>>>>>> the generic fbdev emulation. This patch makes full use of the generic
>>>>>>>>>> fbdev emulation by using its drm_client callbacks. This means that
>>>>>>>>>> drm_mode_config_funcs->output_poll_changed and
>>>>>>>>>> drm_driver->lastclose are
>>>>>>>>>> now handled by the emulation code. Additionally fbdev
>>>>>>>>>> unregister happens
>>>>>>>>>> automatically on drm_dev_unregister().
>>>>>>>>>>
>>>>>>>>>> The drm_fbdev_generic_setup() call is put after
>>>>>>>>>> drm_dev_register() in the
>>>>>>>>>> driver. This is done to highlight the fact that fbdev emulation is an
>>>>>>>>>> internal client that makes use of the driver, it is not part of the
>>>>>>>>>> driver as such. If fbdev setup fails, an error is printed,
>>>>>>>>>> but the driver
>>>>>>>>>> succeeds probing.
>>>>>>>>> I can't really ack/nack this move, on one side it's great to have a
>>>>>>>>> generic fbdev emulation instead of the old fbdev code, but on the
>>>>>>>>> other side the Amlogic platform (like allwinner, samsung, rockchip,
>>>>>>>>> ...)  still relies on an Fbdev variant of the libMali that uses
>>>>>>>>> smem_start...
>>>>>>>>>
>>>>>>>>> I know it's dirty and fbdev based code should be legacy now, but I
>>>>>>>>> consider it like an user-space breakage...
>>>>>>>>>
>>>>>>>>> I'll be happy if ARM provided it's Mali vendors a Fbdev libMali that
>>>>>>>>> could use FBINFO_HIDE_SMEM_START and allocate BO's from the DRM
>>>>>>>>> driver, it won't be the case and it will never be the case until the
>>>>>>>>> Lima projects finalizes.
>>>>>>>>>
>>>>>>>>> So for me it's a no-go until Lima lands upstream in Linux *and* Mesa.
>>>>>>>> My feelings exactly. If the choice is between reducing the DRM driver
>>>>>>>> by a couple of dozens of lines or keeping the mali working, I'll pick
>>>>>>>> the latter, every single day.
>>>>>>> I don't know the reasoning for blocking smem_start other than what Daniel
>>>>>>> wrote in these commit messages:
>>>>>>>
>>>>>>> da6c7707caf3736c1cf968606bd97c07e79625d4
>>>>>>> fbdev: Add FBINFO_HIDE_SMEM_START flag
>>>>>>>
>>>>>>>    DRM drivers really, really, really don't want random userspace to
>>>>>>>    share buffer behind it's back, bypassing the dma-buf buffer sharing
>>>>>>>    machanism. For that reason we've ruthlessly rejected any IOCTL
>>>>>>>    exposing the physical address of any graphics buffer.
>>>>>>>
>>>>>>>    Unfortunately fbdev comes with that built-in. We could just set
>>>>>>>    smem_start to 0, but that means we'd have to hand-roll our own fb_mmap
>>>>>>>    implementation. For good reasons many drivers do that, but
>>>>>>>    smem_start/length is still super convenient.
>>>>>>>
>>>>>>>    Hence instead just stop the leak in the ioctl, to keep fb mmap working
>>>>>>>    as-is. A second patch will set this flag for all drm drivers.
>>>>>>>
>>>>>>>
>>>>>>> 6be8f3bd2c78915a9f3a058a346ae93068d35c01
>>>>>>> drm/fb: Stop leaking physical address
>>>>>>>
>>>>>>>    For buffer sharing, use dma-buf instead. We can't set smem_start to 0
>>>>>>>    unconditionally since that's used by the fbdev mmap default
>>>>>>>    implementation. And we have plenty of userspace which would like to
>>>>>>>    keep that working.
>>>>>>>
>>>>>>>    This might break legit userspace - if it does we need to look at a
>>>>>>>    case-by-cases basis how to handle that. Worst case I expect overrides
>>>>>>>    for only specific drivers, since anything remotely modern should be
>>>>>>>    using dma-buf/prime now (which is about 7 years old now for DRM
>>>>>>>    drivers).
>>>>>>>
>>>>>>>    This issue was uncovered because Noralf's rework to implement a
>>>>>>>    generic fb_probe also implements it's own fb_mmap callback. Which
>>>>>>>    means smem_start didn't have to be set anymore, which blew up some
>>>>>>>    blob in userspace rather badly.
>>>>>>>
>>>>>>>
>>>>>>> Noralf.
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> dri-devel mailing list
>>>>>>> dri-devel@lists.freedesktop.org
>>>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>>>>
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 05/20] drm/meson: Use drm_fbdev_generic_setup()

[Neil Armstrong]

Hi Noralf,

On 17/09/2018 09:53, Neil Armstrong wrote:
> Hi Noralf, Daniel,
> 
> On 14/09/2018 18:33, Noralf Trønnes wrote:
>>
>> Den 14.09.2018 10.23, skrev Neil Armstrong:
>>> Hi Daniel,
>>>
>>> On 13/09/2018 16:55, Daniel Vetter wrote:
>>>> On Thu, Sep 13, 2018 at 04:26:53PM +0200, Neil Armstrong wrote:
>>>>> Hi Daniel,
>>>>>
>>>>> On 13/09/2018 15:21, Daniel Vetter wrote:
>>>>>> On Wed, Sep 12, 2018 at 01:06:07PM +0200, Noralf Trønnes wrote:
>>>>>>> Den 12.09.2018 12.57, skrev Noralf Trønnes:
>>>>>>>> (Cc: Daniel Vetter)
>>>>>>>>
>>>>>>> Somehow that CC was dropped somewhere after leaving email client.
>>>>>>> Trying once more.
>>>>>> Yeah I just made myself unpopular. If you want SMEM_START, then you need
>>>>>> to carry a local patch now. virt_to_pfn on the vmap should work. It's
>>>>>> about 2 lines, including the change to drop HIDE_SMEM_START.
>>>>> Would it be totally unacceptable to add such 2 line :
>>>>> - enabled by a specific config depending on EXPERT and narrowed to specific platforms
>>>>> - printing a big fat pr_warning_once when used
>>>>> - with a big fat comment specifying when this code will be dropped and why we should not activate it
>>>> module_param_debug auto-taints your kernel, I'd just go with that. Plus
>>>> CONFIG_EXPERT (or CONFIG_BROKEN).
>>> OK, I think you mean module_param_unsafe, but I see the point.
>>>
>>>> But yes, I think that's something I'll happily ack. Must be acked by Dave
>>>> (and perhaps a few others too), since defacto this means we now suddenly
>>>> care about closed source blobs. I'd say get someone from amd and a few of
>>>> the driver maintainers on board with this (nouveau, Eric, Rob, Lucas,
>>>> ...), to make sure it really represents community consensus.
>>> I'll drop something, but I'm afraid a kernel won't have this hack, shouldn't this serie be delayed for a release ?
>>
>> It's not this series that drops smem_start support.
>> It happened in commit 894a677f4b3e:
>> drm/cma-helper: Use the generic fbdev emulation
>>
>> This series only deals with the fb_helper callbacks.
>>
>>> @Noaralf, do you have a branch somewhere I can base a work on ?
>>
>> I haven't got a git repo, but you can apply the patches from patchwork:
>>
>> [01/20] drm/fb-helper: Improve error reporting in setup
>> curl https://patchwork.freedesktop.org/patch/247860/mbox/ | git am
>>
>> [05/20] drm/meson: Use drm_fbdev_generic_setup()
>> curl https://patchwork.freedesktop.org/patch/247868/mbox/ | git am

Did you have time reviewing the v2 of "drm/fb_helper: Allow leaking fbdev smem_start" ?

I'll be happy acking this patch once the smem_start stuff gets sorted out.

Thanks,
Neil

[...]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 05/20] drm/meson: Use drm_fbdev_generic_setup()

[Neil Armstrong]

Hi Noralf,

Le 08/09/2018 15:46, Noralf Trønnes a écrit :
> The CMA helper is already using the drm_fb_helper_generic_probe part of
> the generic fbdev emulation. This patch makes full use of the generic
> fbdev emulation by using its drm_client callbacks. This means that
> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
> now handled by the emulation code. Additionally fbdev unregister happens
> automatically on drm_dev_unregister().
> 
> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
> driver. This is done to highlight the fact that fbdev emulation is an
> internal client that makes use of the driver, it is not part of the
> driver as such. If fbdev setup fails, an error is printed, but the driver
> succeeds probing.
> 
> Cc: Neil Armstrong <narmstrong@baylibre.com>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  drivers/gpu/drm/meson/meson_drv.c | 19 ++-----------------
>  drivers/gpu/drm/meson/meson_drv.h |  1 -
>  2 files changed, 2 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> index d3443125e661..348b5a198b9d 100644
> --- a/drivers/gpu/drm/meson/meson_drv.c
> +++ b/drivers/gpu/drm/meson/meson_drv.c
> @@ -68,15 +68,7 @@
>   * - Powering Up HDMI controller and PHY
>   */
>  
> -static void meson_fb_output_poll_changed(struct drm_device *dev)
> -{
> -	struct meson_drm *priv = dev->dev_private;
> -
> -	drm_fbdev_cma_hotplug_event(priv->fbdev);
> -}
> -
>  static const struct drm_mode_config_funcs meson_mode_config_funcs = {
> -	.output_poll_changed = meson_fb_output_poll_changed,
>  	.atomic_check        = drm_atomic_helper_check,
>  	.atomic_commit       = drm_atomic_helper_commit,
>  	.fb_create           = drm_gem_fb_create,
> @@ -282,13 +274,6 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
>  
>  	drm_mode_config_reset(drm);
>  
> -	priv->fbdev = drm_fbdev_cma_init(drm, 32,
> -					 drm->mode_config.num_connector);
> -	if (IS_ERR(priv->fbdev)) {
> -		ret = PTR_ERR(priv->fbdev);
> -		goto free_drm;
> -	}
> -
>  	drm_kms_helper_poll_init(drm);
>  
>  	platform_set_drvdata(pdev, priv);
> @@ -297,6 +282,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
>  	if (ret)
>  		goto free_drm;
>  
> +	drm_fbdev_generic_setup(drm, 32);
> +
>  	return 0;
>  
>  free_drm:
> @@ -313,11 +300,9 @@ static int meson_drv_bind(struct device *dev)
>  static void meson_drv_unbind(struct device *dev)
>  {
>  	struct drm_device *drm = dev_get_drvdata(dev);
> -	struct meson_drm *priv = drm->dev_private;
>  
>  	drm_dev_unregister(drm);
>  	drm_kms_helper_poll_fini(drm);
> -	drm_fbdev_cma_fini(priv->fbdev);
>  	drm_mode_config_cleanup(drm);
>  	drm_dev_put(drm);
>  
> diff --git a/drivers/gpu/drm/meson/meson_drv.h b/drivers/gpu/drm/meson/meson_drv.h
> index 8450d6ac8c9b..aab96260da9f 100644
> --- a/drivers/gpu/drm/meson/meson_drv.h
> +++ b/drivers/gpu/drm/meson/meson_drv.h
> @@ -33,7 +33,6 @@ struct meson_drm {
>  
>  	struct drm_device *drm;
>  	struct drm_crtc *crtc;
> -	struct drm_fbdev_cma *fbdev;
>  	struct drm_plane *primary_plane;
>  
>  	/* Components Data */
> 

A little bit late to get this in 4.20 (or 5.0) and get rid of the old fbdev code,
but at least we have a (dirty) workaround in place for the Mali fbdev blob...

Acked-by: Neil Armstrong <narmstrong@baylibre.com>

Thanks for pushing the fbdev emulation on the right path !

Neil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 05/20] drm/meson: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

Den 03.10.2018 21.24, skrev Neil Armstrong:
> Hi Noralf,
>
> Le 08/09/2018 15:46, Noralf Trønnes a écrit :
>> The CMA helper is already using the drm_fb_helper_generic_probe part of
>> the generic fbdev emulation. This patch makes full use of the generic
>> fbdev emulation by using its drm_client callbacks. This means that
>> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
>> now handled by the emulation code. Additionally fbdev unregister happens
>> automatically on drm_dev_unregister().
>>
>> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
>> driver. This is done to highlight the fact that fbdev emulation is an
>> internal client that makes use of the driver, it is not part of the
>> driver as such. If fbdev setup fails, an error is printed, but the driver
>> succeeds probing.
>>
>> Cc: Neil Armstrong <narmstrong@baylibre.com>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>>   drivers/gpu/drm/meson/meson_drv.c | 19 ++-----------------
>>   drivers/gpu/drm/meson/meson_drv.h |  1 -
>>   2 files changed, 2 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
>> index d3443125e661..348b5a198b9d 100644
>> --- a/drivers/gpu/drm/meson/meson_drv.c
>> +++ b/drivers/gpu/drm/meson/meson_drv.c
>> @@ -68,15 +68,7 @@
>>    * - Powering Up HDMI controller and PHY
>>    */
>>   
>> -static void meson_fb_output_poll_changed(struct drm_device *dev)
>> -{
>> -	struct meson_drm *priv = dev->dev_private;
>> -
>> -	drm_fbdev_cma_hotplug_event(priv->fbdev);
>> -}
>> -
>>   static const struct drm_mode_config_funcs meson_mode_config_funcs = {
>> -	.output_poll_changed = meson_fb_output_poll_changed,
>>   	.atomic_check        = drm_atomic_helper_check,
>>   	.atomic_commit       = drm_atomic_helper_commit,
>>   	.fb_create           = drm_gem_fb_create,
>> @@ -282,13 +274,6 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
>>   
>>   	drm_mode_config_reset(drm);
>>   
>> -	priv->fbdev = drm_fbdev_cma_init(drm, 32,
>> -					 drm->mode_config.num_connector);
>> -	if (IS_ERR(priv->fbdev)) {
>> -		ret = PTR_ERR(priv->fbdev);
>> -		goto free_drm;
>> -	}
>> -
>>   	drm_kms_helper_poll_init(drm);
>>   
>>   	platform_set_drvdata(pdev, priv);
>> @@ -297,6 +282,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
>>   	if (ret)
>>   		goto free_drm;
>>   
>> +	drm_fbdev_generic_setup(drm, 32);
>> +
>>   	return 0;
>>   
>>   free_drm:
>> @@ -313,11 +300,9 @@ static int meson_drv_bind(struct device *dev)
>>   static void meson_drv_unbind(struct device *dev)
>>   {
>>   	struct drm_device *drm = dev_get_drvdata(dev);
>> -	struct meson_drm *priv = drm->dev_private;
>>   
>>   	drm_dev_unregister(drm);
>>   	drm_kms_helper_poll_fini(drm);
>> -	drm_fbdev_cma_fini(priv->fbdev);
>>   	drm_mode_config_cleanup(drm);
>>   	drm_dev_put(drm);
>>   
>> diff --git a/drivers/gpu/drm/meson/meson_drv.h b/drivers/gpu/drm/meson/meson_drv.h
>> index 8450d6ac8c9b..aab96260da9f 100644
>> --- a/drivers/gpu/drm/meson/meson_drv.h
>> +++ b/drivers/gpu/drm/meson/meson_drv.h
>> @@ -33,7 +33,6 @@ struct meson_drm {
>>   
>>   	struct drm_device *drm;
>>   	struct drm_crtc *crtc;
>> -	struct drm_fbdev_cma *fbdev;
>>   	struct drm_plane *primary_plane;
>>   
>>   	/* Components Data */
>>
> A little bit late to get this in 4.20 (or 5.0) and get rid of the old fbdev code,
> but at least we have a (dirty) workaround in place for the Mali fbdev blob...

Glad you found a solution for that!

> Acked-by: Neil Armstrong <narmstrong@baylibre.com>
>
> Thanks for pushing the fbdev emulation on the right path !

Applied to drm-misc-next, thanks.

Noralf.

> Neil

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 06/20] drm/mxsfb: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

The CMA helper is already using the drm_fb_helper_generic_probe part of
the generic fbdev emulation. This patch makes full use of the generic
fbdev emulation by using its drm_client callbacks. This means that
drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
now handled by the emulation code. Additionally fbdev unregister happens
automatically on drm_dev_unregister().

The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
driver. This is done to highlight the fact that fbdev emulation is an
internal client that makes use of the driver, it is not part of the
driver as such. If fbdev setup fails, an error is printed, but the driver
succeeds probing.

Cc: Marek Vasut <marex@denx.de>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/mxsfb/mxsfb_drv.c | 26 ++------------------------
 drivers/gpu/drm/mxsfb/mxsfb_drv.h |  1 -
 2 files changed, 2 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
index ffe5137ccaf8..422827c3d5fd 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
@@ -244,23 +244,12 @@ static int mxsfb_load(struct drm_device *drm, unsigned long flags)
 
 	drm_kms_helper_poll_init(drm);
 
-	mxsfb->fbdev = drm_fbdev_cma_init(drm, 32,
-					  drm->mode_config.num_connector);
-	if (IS_ERR(mxsfb->fbdev)) {
-		ret = PTR_ERR(mxsfb->fbdev);
-		mxsfb->fbdev = NULL;
-		dev_err(drm->dev, "Failed to init FB CMA area\n");
-		goto err_cma;
-	}
-
 	platform_set_drvdata(pdev, drm);
 
 	drm_helper_hpd_irq_event(drm);
 
 	return 0;
 
-err_cma:
-	drm_irq_uninstall(drm);
 err_irq:
 	drm_panel_detach(mxsfb->panel);
 err_vblank:
@@ -271,11 +260,6 @@ static int mxsfb_load(struct drm_device *drm, unsigned long flags)
 
 static void mxsfb_unload(struct drm_device *drm)
 {
-	struct mxsfb_drm_private *mxsfb = drm->dev_private;
-
-	if (mxsfb->fbdev)
-		drm_fbdev_cma_fini(mxsfb->fbdev);
-
 	drm_kms_helper_poll_fini(drm);
 	drm_mode_config_cleanup(drm);
 
@@ -288,13 +272,6 @@ static void mxsfb_unload(struct drm_device *drm)
 	pm_runtime_disable(drm->dev);
 }
 
-static void mxsfb_lastclose(struct drm_device *drm)
-{
-	struct mxsfb_drm_private *mxsfb = drm->dev_private;
-
-	drm_fbdev_cma_restore_mode(mxsfb->fbdev);
-}
-
 static void mxsfb_irq_preinstall(struct drm_device *drm)
 {
 	struct mxsfb_drm_private *mxsfb = drm->dev_private;
@@ -328,7 +305,6 @@ static struct drm_driver mxsfb_driver = {
 	.driver_features	= DRIVER_GEM | DRIVER_MODESET |
 				  DRIVER_PRIME | DRIVER_ATOMIC |
 				  DRIVER_HAVE_IRQ,
-	.lastclose		= mxsfb_lastclose,
 	.irq_handler		= mxsfb_irq_handler,
 	.irq_preinstall		= mxsfb_irq_preinstall,
 	.irq_uninstall		= mxsfb_irq_preinstall,
@@ -393,6 +369,8 @@ static int mxsfb_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_unload;
 
+	drm_fbdev_generic_setup(drm, 32);
+
 	return 0;
 
 err_unload:
diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.h b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
index 5d0883fc805b..bedd6801edca 100644
--- a/drivers/gpu/drm/mxsfb/mxsfb_drv.h
+++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.h
@@ -37,7 +37,6 @@ struct mxsfb_drm_private {
 	struct drm_simple_display_pipe	pipe;
 	struct drm_connector		connector;
 	struct drm_panel		*panel;
-	struct drm_fbdev_cma		*fbdev;
 };
 
 int mxsfb_setup_crtc(struct drm_device *dev);
-- 
2.15.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 07/20] drm/rcar-du: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

The CMA helper is already using the drm_fb_helper_generic_probe part of
the generic fbdev emulation. This patch makes full use of the generic
fbdev emulation by using its drm_client callbacks. This means that
drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
now handled by the emulation code. Additionally fbdev unregister happens
automatically on drm_dev_unregister().

The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
driver. This is done to highlight the fact that fbdev emulation is an
internal client that makes use of the driver, it is not part of the
driver as such. If fbdev setup fails, an error is printed, but the driver
succeeds probing.

Use the drm_mode_config_helper_suspend/resume helpers instead of open
coding it.

drm_fbdev_generic_setup() handles mode_config.num_connector being zero.
In that case it retries fbdev setup on the next .output_poll_changed.

Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/rcar-du/rcar_du_drv.c | 34 ++++++----------------------------
 drivers/gpu/drm/rcar-du/rcar_du_drv.h |  3 ---
 drivers/gpu/drm/rcar-du/rcar_du_kms.c | 21 ---------------------
 3 files changed, 6 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
index 02aee6cb0e53..bcb01d7b4a89 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
@@ -25,7 +25,9 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_fb_cma_helper.h>
+#include <drm/drm_fb_helper.h>
 #include <drm/drm_gem_cma_helper.h>
+#include <drm/drm_modeset_helper.h>
 
 #include "rcar_du_drv.h"
 #include "rcar_du_kms.h"
@@ -316,19 +318,11 @@ MODULE_DEVICE_TABLE(of, rcar_du_of_table);
  * DRM operations
  */
 
-static void rcar_du_lastclose(struct drm_device *dev)
-{
-	struct rcar_du_device *rcdu = dev->dev_private;
-
-	drm_fbdev_cma_restore_mode(rcdu->fbdev);
-}
-
 DEFINE_DRM_GEM_CMA_FOPS(rcar_du_fops);
 
 static struct drm_driver rcar_du_driver = {
 	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME
 				| DRIVER_ATOMIC,
-	.lastclose		= rcar_du_lastclose,
 	.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,
@@ -357,30 +351,15 @@ static struct drm_driver rcar_du_driver = {
 static int rcar_du_pm_suspend(struct device *dev)
 {
 	struct rcar_du_device *rcdu = dev_get_drvdata(dev);
-	struct drm_atomic_state *state;
 
-	drm_kms_helper_poll_disable(rcdu->ddev);
-	drm_fbdev_cma_set_suspend_unlocked(rcdu->fbdev, true);
-
-	state = drm_atomic_helper_suspend(rcdu->ddev);
-	if (IS_ERR(state)) {
-		drm_fbdev_cma_set_suspend_unlocked(rcdu->fbdev, false);
-		drm_kms_helper_poll_enable(rcdu->ddev);
-		return PTR_ERR(state);
-	}
-
-	rcdu->suspend_state = state;
-
-	return 0;
+	return drm_mode_config_helper_suspend(rcdu->ddev);
 }
 
 static int rcar_du_pm_resume(struct device *dev)
 {
 	struct rcar_du_device *rcdu = dev_get_drvdata(dev);
 
-	drm_atomic_helper_resume(rcdu->ddev, rcdu->suspend_state);
-	drm_fbdev_cma_set_suspend_unlocked(rcdu->fbdev, false);
-	drm_kms_helper_poll_enable(rcdu->ddev);
+	drm_mode_config_helper_resume(rcdu->ddev);
 
 	return 0;
 }
@@ -401,9 +380,6 @@ static int rcar_du_remove(struct platform_device *pdev)
 
 	drm_dev_unregister(ddev);
 
-	if (rcdu->fbdev)
-		drm_fbdev_cma_fini(rcdu->fbdev);
-
 	drm_kms_helper_poll_fini(ddev);
 	drm_mode_config_cleanup(ddev);
 
@@ -463,6 +439,8 @@ static int rcar_du_probe(struct platform_device *pdev)
 
 	DRM_INFO("Device %s probed\n", dev_name(&pdev->dev));
 
+	drm_fbdev_generic_setup(ddev, 32);
+
 	return 0;
 
 error:
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
index b3a25e8e07d0..58d5c730d2d2 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
@@ -24,7 +24,6 @@
 struct clk;
 struct device;
 struct drm_device;
-struct drm_fbdev_cma;
 struct rcar_du_device;
 
 #define RCAR_DU_FEATURE_CRTC_IRQ_CLOCK	(1 << 0)	/* Per-CRTC IRQ and clock */
@@ -77,8 +76,6 @@ struct rcar_du_device {
 	void __iomem *mmio;
 
 	struct drm_device *ddev;
-	struct drm_fbdev_cma *fbdev;
-	struct drm_atomic_state *suspend_state;
 
 	struct rcar_du_crtc crtcs[RCAR_DU_MAX_CRTCS];
 	unsigned int num_crtcs;
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
index f0bc7cc0e913..19dfe4bf5446 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
@@ -216,13 +216,6 @@ rcar_du_fb_create(struct drm_device *dev, struct drm_file *file_priv,
 	return drm_gem_fb_create(dev, file_priv, mode_cmd);
 }
 
-static void rcar_du_output_poll_changed(struct drm_device *dev)
-{
-	struct rcar_du_device *rcdu = dev->dev_private;
-
-	drm_fbdev_cma_hotplug_event(rcdu->fbdev);
-}
-
 /* -----------------------------------------------------------------------------
  * Atomic Check and Update
  */
@@ -269,7 +262,6 @@ static const struct drm_mode_config_helper_funcs rcar_du_mode_config_helper = {
 
 static const struct drm_mode_config_funcs rcar_du_mode_config_funcs = {
 	.fb_create = rcar_du_fb_create,
-	.output_poll_changed = rcar_du_output_poll_changed,
 	.atomic_check = rcar_du_atomic_check,
 	.atomic_commit = drm_atomic_helper_commit,
 };
@@ -504,7 +496,6 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu)
 
 	struct drm_device *dev = rcdu->ddev;
 	struct drm_encoder *encoder;
-	struct drm_fbdev_cma *fbdev;
 	unsigned int num_encoders;
 	unsigned int num_groups;
 	unsigned int swindex;
@@ -621,17 +612,5 @@ 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_connector);
-		if (IS_ERR(fbdev))
-			return PTR_ERR(fbdev);
-
-		rcdu->fbdev = fbdev;
-	} else {
-		dev_info(rcdu->dev,
-			 "no connector found, disabling fbdev emulation\n");
-	}
-
 	return 0;
 }
-- 
2.15.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 07/20] drm/rcar-du: Use drm_fbdev_generic_setup()

[Sam Ravnborg]

On Sat, Sep 08, 2018 at 03:46:35PM +0200, Noralf Trønnes wrote:
> The CMA helper is already using the drm_fb_helper_generic_probe part of
> the generic fbdev emulation. This patch makes full use of the generic
> fbdev emulation by using its drm_client callbacks. This means that
> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
> now handled by the emulation code. Additionally fbdev unregister happens
> automatically on drm_dev_unregister().
> 
> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
> driver. This is done to highlight the fact that fbdev emulation is an
> internal client that makes use of the driver, it is not part of the
> driver as such. If fbdev setup fails, an error is printed, but the driver
> succeeds probing.
> 
> Use the drm_mode_config_helper_suspend/resume helpers instead of open
> coding it.
Seems a bit un-related to the rest of the patch.
But the changes looks fine (from my limited DRM experience that is).

	Sam
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 07/20] drm/rcar-du: Use drm_fbdev_generic_setup()

[Laurent Pinchart]

Hi Noralf,

Thank you for the patch.

On Saturday, 8 September 2018 16:46:35 EEST Noralf Trønnes wrote:
> The CMA helper is already using the drm_fb_helper_generic_probe part of
> the generic fbdev emulation. This patch makes full use of the generic
> fbdev emulation by using its drm_client callbacks. This means that
> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
> now handled by the emulation code. Additionally fbdev unregister happens
> automatically on drm_dev_unregister().
> 
> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
> driver. This is done to highlight the fact that fbdev emulation is an
> internal client that makes use of the driver, it is not part of the
> driver as such. If fbdev setup fails, an error is printed, but the driver
> succeeds probing.
> 
> Use the drm_mode_config_helper_suspend/resume helpers instead of open
> coding it.

As Sam already pointed out, shouldn't this change be split to a separate patch 
?

Apart from that this patch looks good to me.

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

I assume you will push the entire series through drm-misc. If you'd like me to 
take this patch in my tree instead please let me know.

> drm_fbdev_generic_setup() handles mode_config.num_connector being zero.
> In that case it retries fbdev setup on the next .output_poll_changed.
> 
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c | 34 ++++++--------------------------
>  drivers/gpu/drm/rcar-du/rcar_du_drv.h |  3 ---
>  drivers/gpu/drm/rcar-du/rcar_du_kms.c | 21 ---------------------
>  3 files changed, 6 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 02aee6cb0e53..bcb01d7b4a89
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -25,7 +25,9 @@
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_crtc_helper.h>
>  #include <drm/drm_fb_cma_helper.h>
> +#include <drm/drm_fb_helper.h>
>  #include <drm/drm_gem_cma_helper.h>
> +#include <drm/drm_modeset_helper.h>
> 
>  #include "rcar_du_drv.h"
>  #include "rcar_du_kms.h"
> @@ -316,19 +318,11 @@ MODULE_DEVICE_TABLE(of, rcar_du_of_table);
>   * DRM operations
>   */
> 
> -static void rcar_du_lastclose(struct drm_device *dev)
> -{
> -	struct rcar_du_device *rcdu = dev->dev_private;
> -
> -	drm_fbdev_cma_restore_mode(rcdu->fbdev);
> -}
> -
>  DEFINE_DRM_GEM_CMA_FOPS(rcar_du_fops);
> 
>  static struct drm_driver rcar_du_driver = {
>  	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME
> 
>  				| DRIVER_ATOMIC,
> 
> -	.lastclose		= rcar_du_lastclose,
>  	.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,
> @@ -357,30 +351,15 @@ static struct drm_driver rcar_du_driver = {
>  static int rcar_du_pm_suspend(struct device *dev)
>  {
>  	struct rcar_du_device *rcdu = dev_get_drvdata(dev);
> -	struct drm_atomic_state *state;
> 
> -	drm_kms_helper_poll_disable(rcdu->ddev);
> -	drm_fbdev_cma_set_suspend_unlocked(rcdu->fbdev, true);
> -
> -	state = drm_atomic_helper_suspend(rcdu->ddev);
> -	if (IS_ERR(state)) {
> -		drm_fbdev_cma_set_suspend_unlocked(rcdu->fbdev, false);
> -		drm_kms_helper_poll_enable(rcdu->ddev);
> -		return PTR_ERR(state);
> -	}
> -
> -	rcdu->suspend_state = state;
> -
> -	return 0;
> +	return drm_mode_config_helper_suspend(rcdu->ddev);
>  }
> 
>  static int rcar_du_pm_resume(struct device *dev)
>  {
>  	struct rcar_du_device *rcdu = dev_get_drvdata(dev);
> 
> -	drm_atomic_helper_resume(rcdu->ddev, rcdu->suspend_state);
> -	drm_fbdev_cma_set_suspend_unlocked(rcdu->fbdev, false);
> -	drm_kms_helper_poll_enable(rcdu->ddev);
> +	drm_mode_config_helper_resume(rcdu->ddev);
> 
>  	return 0;
>  }
> @@ -401,9 +380,6 @@ static int rcar_du_remove(struct platform_device *pdev)
> 
>  	drm_dev_unregister(ddev);
> 
> -	if (rcdu->fbdev)
> -		drm_fbdev_cma_fini(rcdu->fbdev);
> -
>  	drm_kms_helper_poll_fini(ddev);
>  	drm_mode_config_cleanup(ddev);
> 
> @@ -463,6 +439,8 @@ static int rcar_du_probe(struct platform_device *pdev)
> 
>  	DRM_INFO("Device %s probed\n", dev_name(&pdev->dev));
> 
> +	drm_fbdev_generic_setup(ddev, 32);
> +
>  	return 0;
> 
>  error:
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> b/drivers/gpu/drm/rcar-du/rcar_du_drv.h index b3a25e8e07d0..58d5c730d2d2
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> @@ -24,7 +24,6 @@
>  struct clk;
>  struct device;
>  struct drm_device;
> -struct drm_fbdev_cma;
>  struct rcar_du_device;
> 
>  #define RCAR_DU_FEATURE_CRTC_IRQ_CLOCK	(1 << 0)	/* Per-CRTC IRQ and clock
> */ @@ -77,8 +76,6 @@ struct rcar_du_device {
>  	void __iomem *mmio;
> 
>  	struct drm_device *ddev;
> -	struct drm_fbdev_cma *fbdev;
> -	struct drm_atomic_state *suspend_state;
> 
>  	struct rcar_du_crtc crtcs[RCAR_DU_MAX_CRTCS];
>  	unsigned int num_crtcs;
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index f0bc7cc0e913..19dfe4bf5446
> 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> @@ -216,13 +216,6 @@ rcar_du_fb_create(struct drm_device *dev, struct
> drm_file *file_priv, return drm_gem_fb_create(dev, file_priv, mode_cmd);
>  }
> 
> -static void rcar_du_output_poll_changed(struct drm_device *dev)
> -{
> -	struct rcar_du_device *rcdu = dev->dev_private;
> -
> -	drm_fbdev_cma_hotplug_event(rcdu->fbdev);
> -}
> -
>  /*
> ---------------------------------------------------------------------------
> -- * Atomic Check and Update
>   */
> @@ -269,7 +262,6 @@ static const struct drm_mode_config_helper_funcs
> rcar_du_mode_config_helper = {
> 
>  static const struct drm_mode_config_funcs rcar_du_mode_config_funcs = {
>  	.fb_create = rcar_du_fb_create,
> -	.output_poll_changed = rcar_du_output_poll_changed,
>  	.atomic_check = rcar_du_atomic_check,
>  	.atomic_commit = drm_atomic_helper_commit,
>  };
> @@ -504,7 +496,6 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu)
> 
>  	struct drm_device *dev = rcdu->ddev;
>  	struct drm_encoder *encoder;
> -	struct drm_fbdev_cma *fbdev;
>  	unsigned int num_encoders;
>  	unsigned int num_groups;
>  	unsigned int swindex;
> @@ -621,17 +612,5 @@ 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_connector);
> -		if (IS_ERR(fbdev))
> -			return PTR_ERR(fbdev);
> -
> -		rcdu->fbdev = fbdev;
> -	} else {
> -		dev_info(rcdu->dev,
> -			 "no connector found, disabling fbdev emulation\n");
> -	}
> -
>  	return 0;
>  }


-- 
Regards,

Laurent Pinchart



_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 07/20] drm/rcar-du: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

Den 09.09.2018 16.13, skrev Laurent Pinchart:
> Hi Noralf,
>
> Thank you for the patch.
>
> On Saturday, 8 September 2018 16:46:35 EEST Noralf Trønnes wrote:
>> The CMA helper is already using the drm_fb_helper_generic_probe part of
>> the generic fbdev emulation. This patch makes full use of the generic
>> fbdev emulation by using its drm_client callbacks. This means that
>> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
>> now handled by the emulation code. Additionally fbdev unregister happens
>> automatically on drm_dev_unregister().
>>
>> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
>> driver. This is done to highlight the fact that fbdev emulation is an
>> internal client that makes use of the driver, it is not part of the
>> driver as such. If fbdev setup fails, an error is printed, but the driver
>> succeeds probing.
>>
>> Use the drm_mode_config_helper_suspend/resume helpers instead of open
>> coding it.
> As Sam already pointed out, shouldn't this change be split to a separate patch
> ?

I can split it up in the next round.

> Apart from that this patch looks good to me.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> I assume you will push the entire series through drm-misc. If you'd like me to
> take this patch in my tree instead please let me know.

Yes I plan to do that unless someone specifically says that they'll take
through their tree.

Noralf.

>> drm_fbdev_generic_setup() handles mode_config.num_connector being zero.
>> In that case it retries fbdev setup on the next .output_poll_changed.
>>
>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>>   drivers/gpu/drm/rcar-du/rcar_du_drv.c | 34 ++++++--------------------------
>>   drivers/gpu/drm/rcar-du/rcar_du_drv.h |  3 ---
>>   drivers/gpu/drm/rcar-du/rcar_du_kms.c | 21 ---------------------
>>   3 files changed, 6 insertions(+), 52 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>> b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 02aee6cb0e53..bcb01d7b4a89
>> 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
>> @@ -25,7 +25,9 @@
>>   #include <drm/drm_atomic_helper.h>
>>   #include <drm/drm_crtc_helper.h>
>>   #include <drm/drm_fb_cma_helper.h>
>> +#include <drm/drm_fb_helper.h>
>>   #include <drm/drm_gem_cma_helper.h>
>> +#include <drm/drm_modeset_helper.h>
>>
>>   #include "rcar_du_drv.h"
>>   #include "rcar_du_kms.h"
>> @@ -316,19 +318,11 @@ MODULE_DEVICE_TABLE(of, rcar_du_of_table);
>>    * DRM operations
>>    */
>>
>> -static void rcar_du_lastclose(struct drm_device *dev)
>> -{
>> -	struct rcar_du_device *rcdu = dev->dev_private;
>> -
>> -	drm_fbdev_cma_restore_mode(rcdu->fbdev);
>> -}
>> -
>>   DEFINE_DRM_GEM_CMA_FOPS(rcar_du_fops);
>>
>>   static struct drm_driver rcar_du_driver = {
>>   	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME
>>
>>   				| DRIVER_ATOMIC,
>>
>> -	.lastclose		= rcar_du_lastclose,
>>   	.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,
>> @@ -357,30 +351,15 @@ static struct drm_driver rcar_du_driver = {
>>   static int rcar_du_pm_suspend(struct device *dev)
>>   {
>>   	struct rcar_du_device *rcdu = dev_get_drvdata(dev);
>> -	struct drm_atomic_state *state;
>>
>> -	drm_kms_helper_poll_disable(rcdu->ddev);
>> -	drm_fbdev_cma_set_suspend_unlocked(rcdu->fbdev, true);
>> -
>> -	state = drm_atomic_helper_suspend(rcdu->ddev);
>> -	if (IS_ERR(state)) {
>> -		drm_fbdev_cma_set_suspend_unlocked(rcdu->fbdev, false);
>> -		drm_kms_helper_poll_enable(rcdu->ddev);
>> -		return PTR_ERR(state);
>> -	}
>> -
>> -	rcdu->suspend_state = state;
>> -
>> -	return 0;
>> +	return drm_mode_config_helper_suspend(rcdu->ddev);
>>   }
>>
>>   static int rcar_du_pm_resume(struct device *dev)
>>   {
>>   	struct rcar_du_device *rcdu = dev_get_drvdata(dev);
>>
>> -	drm_atomic_helper_resume(rcdu->ddev, rcdu->suspend_state);
>> -	drm_fbdev_cma_set_suspend_unlocked(rcdu->fbdev, false);
>> -	drm_kms_helper_poll_enable(rcdu->ddev);
>> +	drm_mode_config_helper_resume(rcdu->ddev);
>>
>>   	return 0;
>>   }
>> @@ -401,9 +380,6 @@ static int rcar_du_remove(struct platform_device *pdev)
>>
>>   	drm_dev_unregister(ddev);
>>
>> -	if (rcdu->fbdev)
>> -		drm_fbdev_cma_fini(rcdu->fbdev);
>> -
>>   	drm_kms_helper_poll_fini(ddev);
>>   	drm_mode_config_cleanup(ddev);
>>
>> @@ -463,6 +439,8 @@ static int rcar_du_probe(struct platform_device *pdev)
>>
>>   	DRM_INFO("Device %s probed\n", dev_name(&pdev->dev));
>>
>> +	drm_fbdev_generic_setup(ddev, 32);
>> +
>>   	return 0;
>>
>>   error:
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
>> b/drivers/gpu/drm/rcar-du/rcar_du_drv.h index b3a25e8e07d0..58d5c730d2d2
>> 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
>> @@ -24,7 +24,6 @@
>>   struct clk;
>>   struct device;
>>   struct drm_device;
>> -struct drm_fbdev_cma;
>>   struct rcar_du_device;
>>
>>   #define RCAR_DU_FEATURE_CRTC_IRQ_CLOCK	(1 << 0)	/* Per-CRTC IRQ and clock
>> */ @@ -77,8 +76,6 @@ struct rcar_du_device {
>>   	void __iomem *mmio;
>>
>>   	struct drm_device *ddev;
>> -	struct drm_fbdev_cma *fbdev;
>> -	struct drm_atomic_state *suspend_state;
>>
>>   	struct rcar_du_crtc crtcs[RCAR_DU_MAX_CRTCS];
>>   	unsigned int num_crtcs;
>> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>> b/drivers/gpu/drm/rcar-du/rcar_du_kms.c index f0bc7cc0e913..19dfe4bf5446
>> 100644
>> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
>> @@ -216,13 +216,6 @@ rcar_du_fb_create(struct drm_device *dev, struct
>> drm_file *file_priv, return drm_gem_fb_create(dev, file_priv, mode_cmd);
>>   }
>>
>> -static void rcar_du_output_poll_changed(struct drm_device *dev)
>> -{
>> -	struct rcar_du_device *rcdu = dev->dev_private;
>> -
>> -	drm_fbdev_cma_hotplug_event(rcdu->fbdev);
>> -}
>> -
>>   /*
>> ---------------------------------------------------------------------------
>> -- * Atomic Check and Update
>>    */
>> @@ -269,7 +262,6 @@ static const struct drm_mode_config_helper_funcs
>> rcar_du_mode_config_helper = {
>>
>>   static const struct drm_mode_config_funcs rcar_du_mode_config_funcs = {
>>   	.fb_create = rcar_du_fb_create,
>> -	.output_poll_changed = rcar_du_output_poll_changed,
>>   	.atomic_check = rcar_du_atomic_check,
>>   	.atomic_commit = drm_atomic_helper_commit,
>>   };
>> @@ -504,7 +496,6 @@ int rcar_du_modeset_init(struct rcar_du_device *rcdu)
>>
>>   	struct drm_device *dev = rcdu->ddev;
>>   	struct drm_encoder *encoder;
>> -	struct drm_fbdev_cma *fbdev;
>>   	unsigned int num_encoders;
>>   	unsigned int num_groups;
>>   	unsigned int swindex;
>> @@ -621,17 +612,5 @@ 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_connector);
>> -		if (IS_ERR(fbdev))
>> -			return PTR_ERR(fbdev);
>> -
>> -		rcdu->fbdev = fbdev;
>> -	} else {
>> -		dev_info(rcdu->dev,
>> -			 "no connector found, disabling fbdev emulation\n");
>> -	}
>> -
>>   	return 0;
>>   }
>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 08/20] drm/arm/hdlcd: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

The CMA helper is already using the drm_fb_helper_generic_probe part of
the generic fbdev emulation. This patch makes full use of the generic
fbdev emulation by using its drm_client callbacks. This means that
drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
now handled by the emulation code. Additionally fbdev unregister happens
automatically on drm_dev_unregister().

The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
driver. This is done to highlight the fact that fbdev emulation is an
internal client that makes use of the driver, it is not part of the
driver as such. If fbdev setup fails, an error is printed, but the driver
succeeds probing.

Cc: Liviu Dudau <liviu.dudau@arm.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/arm/hdlcd_drv.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
index 0ed1cde98cf8..dfad8d06d108 100644
--- a/drivers/gpu/drm/arm/hdlcd_drv.c
+++ b/drivers/gpu/drm/arm/hdlcd_drv.c
@@ -103,7 +103,6 @@ static int hdlcd_load(struct drm_device *drm, unsigned long flags)
 
 static const struct drm_mode_config_funcs hdlcd_mode_config_funcs = {
 	.fb_create = drm_gem_fb_create,
-	.output_poll_changed = drm_fb_helper_output_poll_changed,
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = drm_atomic_helper_commit,
 };
@@ -233,7 +232,6 @@ static struct drm_driver hdlcd_driver = {
 	.driver_features = DRIVER_HAVE_IRQ | DRIVER_GEM |
 			   DRIVER_MODESET | DRIVER_PRIME |
 			   DRIVER_ATOMIC,
-	.lastclose = drm_fb_helper_lastclose,
 	.irq_handler = hdlcd_irq,
 	.irq_preinstall = hdlcd_irq_preinstall,
 	.irq_postinstall = hdlcd_irq_postinstall,
@@ -308,19 +306,15 @@ static int hdlcd_drm_bind(struct device *dev)
 	drm_mode_config_reset(drm);
 	drm_kms_helper_poll_init(drm);
 
-	ret = drm_fb_cma_fbdev_init(drm, 32, 0);
-	if (ret)
-		goto err_fbdev;
-
 	ret = drm_dev_register(drm, 0);
 	if (ret)
 		goto err_register;
 
+	drm_fbdev_generic_setup(drm, 32);
+
 	return 0;
 
 err_register:
-	drm_fb_cma_fbdev_fini(drm);
-err_fbdev:
 	drm_kms_helper_poll_fini(drm);
 err_vblank:
 	pm_runtime_disable(drm->dev);
@@ -346,7 +340,6 @@ static void hdlcd_drm_unbind(struct device *dev)
 	struct hdlcd_drm_private *hdlcd = drm->dev_private;
 
 	drm_dev_unregister(drm);
-	drm_fb_cma_fbdev_fini(drm);
 	drm_kms_helper_poll_fini(drm);
 	component_unbind_all(dev, drm);
 	of_node_put(hdlcd->crtc.port);
-- 
2.15.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 08/20] drm/arm/hdlcd: Use drm_fbdev_generic_setup()

[Liviu Dudau]

On Sat, Sep 08, 2018 at 03:46:36PM +0200, Noralf Trønnes wrote:
> The CMA helper is already using the drm_fb_helper_generic_probe part of
> the generic fbdev emulation. This patch makes full use of the generic
> fbdev emulation by using its drm_client callbacks. This means that
> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
> now handled by the emulation code. Additionally fbdev unregister happens
> automatically on drm_dev_unregister().
> 
> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
> driver. This is done to highlight the fact that fbdev emulation is an
> internal client that makes use of the driver, it is not part of the
> driver as such. If fbdev setup fails, an error is printed, but the driver
> succeeds probing.
> 
> Cc: Liviu Dudau <liviu.dudau@arm.com>

Acked-by: Liviu Dudau <liviu.dudau@arm.com>

I'm assuming you're going to push the whole series through drm-misc-next rather
than me picking up this patch into the HDLCD tree, if that is fine with you.

Best regards,
Liviu

> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  drivers/gpu/drm/arm/hdlcd_drv.c | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
> index 0ed1cde98cf8..dfad8d06d108 100644
> --- a/drivers/gpu/drm/arm/hdlcd_drv.c
> +++ b/drivers/gpu/drm/arm/hdlcd_drv.c
> @@ -103,7 +103,6 @@ static int hdlcd_load(struct drm_device *drm, unsigned long flags)
>  
>  static const struct drm_mode_config_funcs hdlcd_mode_config_funcs = {
>  	.fb_create = drm_gem_fb_create,
> -	.output_poll_changed = drm_fb_helper_output_poll_changed,
>  	.atomic_check = drm_atomic_helper_check,
>  	.atomic_commit = drm_atomic_helper_commit,
>  };
> @@ -233,7 +232,6 @@ static struct drm_driver hdlcd_driver = {
>  	.driver_features = DRIVER_HAVE_IRQ | DRIVER_GEM |
>  			   DRIVER_MODESET | DRIVER_PRIME |
>  			   DRIVER_ATOMIC,
> -	.lastclose = drm_fb_helper_lastclose,
>  	.irq_handler = hdlcd_irq,
>  	.irq_preinstall = hdlcd_irq_preinstall,
>  	.irq_postinstall = hdlcd_irq_postinstall,
> @@ -308,19 +306,15 @@ static int hdlcd_drm_bind(struct device *dev)
>  	drm_mode_config_reset(drm);
>  	drm_kms_helper_poll_init(drm);
>  
> -	ret = drm_fb_cma_fbdev_init(drm, 32, 0);
> -	if (ret)
> -		goto err_fbdev;
> -
>  	ret = drm_dev_register(drm, 0);
>  	if (ret)
>  		goto err_register;
>  
> +	drm_fbdev_generic_setup(drm, 32);
> +
>  	return 0;
>  
>  err_register:
> -	drm_fb_cma_fbdev_fini(drm);
> -err_fbdev:
>  	drm_kms_helper_poll_fini(drm);
>  err_vblank:
>  	pm_runtime_disable(drm->dev);
> @@ -346,7 +340,6 @@ static void hdlcd_drm_unbind(struct device *dev)
>  	struct hdlcd_drm_private *hdlcd = drm->dev_private;
>  
>  	drm_dev_unregister(drm);
> -	drm_fb_cma_fbdev_fini(drm);
>  	drm_kms_helper_poll_fini(drm);
>  	component_unbind_all(dev, drm);
>  	of_node_put(hdlcd->crtc.port);
> -- 
> 2.15.1
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 08/20] drm/arm/hdlcd: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

Den 11.09.2018 14.17, skrev Liviu Dudau:
> On Sat, Sep 08, 2018 at 03:46:36PM +0200, Noralf Trønnes wrote:
>> The CMA helper is already using the drm_fb_helper_generic_probe part of
>> the generic fbdev emulation. This patch makes full use of the generic
>> fbdev emulation by using its drm_client callbacks. This means that
>> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
>> now handled by the emulation code. Additionally fbdev unregister happens
>> automatically on drm_dev_unregister().
>>
>> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
>> driver. This is done to highlight the fact that fbdev emulation is an
>> internal client that makes use of the driver, it is not part of the
>> driver as such. If fbdev setup fails, an error is printed, but the driver
>> succeeds probing.
>>
>> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Acked-by: Liviu Dudau <liviu.dudau@arm.com>
>
> I'm assuming you're going to push the whole series through drm-misc-next rather
> than me picking up this patch into the HDLCD tree, if that is fine with you.

I'll take them through drm-misc-next.

Thanks,
Noralf.

> Best regards,
> Liviu
>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>>   drivers/gpu/drm/arm/hdlcd_drv.c | 11 ++---------
>>   1 file changed, 2 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
>> index 0ed1cde98cf8..dfad8d06d108 100644
>> --- a/drivers/gpu/drm/arm/hdlcd_drv.c
>> +++ b/drivers/gpu/drm/arm/hdlcd_drv.c
>> @@ -103,7 +103,6 @@ static int hdlcd_load(struct drm_device *drm, unsigned long flags)
>>   
>>   static const struct drm_mode_config_funcs hdlcd_mode_config_funcs = {
>>   	.fb_create = drm_gem_fb_create,
>> -	.output_poll_changed = drm_fb_helper_output_poll_changed,
>>   	.atomic_check = drm_atomic_helper_check,
>>   	.atomic_commit = drm_atomic_helper_commit,
>>   };
>> @@ -233,7 +232,6 @@ static struct drm_driver hdlcd_driver = {
>>   	.driver_features = DRIVER_HAVE_IRQ | DRIVER_GEM |
>>   			   DRIVER_MODESET | DRIVER_PRIME |
>>   			   DRIVER_ATOMIC,
>> -	.lastclose = drm_fb_helper_lastclose,
>>   	.irq_handler = hdlcd_irq,
>>   	.irq_preinstall = hdlcd_irq_preinstall,
>>   	.irq_postinstall = hdlcd_irq_postinstall,
>> @@ -308,19 +306,15 @@ static int hdlcd_drm_bind(struct device *dev)
>>   	drm_mode_config_reset(drm);
>>   	drm_kms_helper_poll_init(drm);
>>   
>> -	ret = drm_fb_cma_fbdev_init(drm, 32, 0);
>> -	if (ret)
>> -		goto err_fbdev;
>> -
>>   	ret = drm_dev_register(drm, 0);
>>   	if (ret)
>>   		goto err_register;
>>   
>> +	drm_fbdev_generic_setup(drm, 32);
>> +
>>   	return 0;
>>   
>>   err_register:
>> -	drm_fb_cma_fbdev_fini(drm);
>> -err_fbdev:
>>   	drm_kms_helper_poll_fini(drm);
>>   err_vblank:
>>   	pm_runtime_disable(drm->dev);
>> @@ -346,7 +340,6 @@ static void hdlcd_drm_unbind(struct device *dev)
>>   	struct hdlcd_drm_private *hdlcd = drm->dev_private;
>>   
>>   	drm_dev_unregister(drm);
>> -	drm_fb_cma_fbdev_fini(drm);
>>   	drm_kms_helper_poll_fini(drm);
>>   	component_unbind_all(dev, drm);
>>   	of_node_put(hdlcd->crtc.port);
>> -- 
>> 2.15.1
>>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 08/20] drm/arm/hdlcd: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

Den 11.09.2018 14.17, skrev Liviu Dudau:
> On Sat, Sep 08, 2018 at 03:46:36PM +0200, Noralf Trønnes wrote:
>> The CMA helper is already using the drm_fb_helper_generic_probe part of
>> the generic fbdev emulation. This patch makes full use of the generic
>> fbdev emulation by using its drm_client callbacks. This means that
>> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
>> now handled by the emulation code. Additionally fbdev unregister happens
>> automatically on drm_dev_unregister().
>>
>> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
>> driver. This is done to highlight the fact that fbdev emulation is an
>> internal client that makes use of the driver, it is not part of the
>> driver as such. If fbdev setup fails, an error is printed, but the driver
>> succeeds probing.
>>
>> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Acked-by: Liviu Dudau <liviu.dudau@arm.com>
>
> I'm assuming you're going to push the whole series through drm-misc-next rather
> than me picking up this patch into the HDLCD tree, if that is fine with you.

Applied to drm-misc-next, thanks.

Noralf.

> Best regards,
> Liviu
>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>>   drivers/gpu/drm/arm/hdlcd_drv.c | 11 ++---------
>>   1 file changed, 2 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
>> index 0ed1cde98cf8..dfad8d06d108 100644
>> --- a/drivers/gpu/drm/arm/hdlcd_drv.c
>> +++ b/drivers/gpu/drm/arm/hdlcd_drv.c
>> @@ -103,7 +103,6 @@ static int hdlcd_load(struct drm_device *drm, unsigned long flags)
>>   
>>   static const struct drm_mode_config_funcs hdlcd_mode_config_funcs = {
>>   	.fb_create = drm_gem_fb_create,
>> -	.output_poll_changed = drm_fb_helper_output_poll_changed,
>>   	.atomic_check = drm_atomic_helper_check,
>>   	.atomic_commit = drm_atomic_helper_commit,
>>   };
>> @@ -233,7 +232,6 @@ static struct drm_driver hdlcd_driver = {
>>   	.driver_features = DRIVER_HAVE_IRQ | DRIVER_GEM |
>>   			   DRIVER_MODESET | DRIVER_PRIME |
>>   			   DRIVER_ATOMIC,
>> -	.lastclose = drm_fb_helper_lastclose,
>>   	.irq_handler = hdlcd_irq,
>>   	.irq_preinstall = hdlcd_irq_preinstall,
>>   	.irq_postinstall = hdlcd_irq_postinstall,
>> @@ -308,19 +306,15 @@ static int hdlcd_drm_bind(struct device *dev)
>>   	drm_mode_config_reset(drm);
>>   	drm_kms_helper_poll_init(drm);
>>   
>> -	ret = drm_fb_cma_fbdev_init(drm, 32, 0);
>> -	if (ret)
>> -		goto err_fbdev;
>> -
>>   	ret = drm_dev_register(drm, 0);
>>   	if (ret)
>>   		goto err_register;
>>   
>> +	drm_fbdev_generic_setup(drm, 32);
>> +
>>   	return 0;
>>   
>>   err_register:
>> -	drm_fb_cma_fbdev_fini(drm);
>> -err_fbdev:
>>   	drm_kms_helper_poll_fini(drm);
>>   err_vblank:
>>   	pm_runtime_disable(drm->dev);
>> @@ -346,7 +340,6 @@ static void hdlcd_drm_unbind(struct device *dev)
>>   	struct hdlcd_drm_private *hdlcd = drm->dev_private;
>>   
>>   	drm_dev_unregister(drm);
>> -	drm_fb_cma_fbdev_fini(drm);
>>   	drm_kms_helper_poll_fini(drm);
>>   	component_unbind_all(dev, drm);
>>   	of_node_put(hdlcd->crtc.port);
>> -- 
>> 2.15.1
>>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 09/20] drm/arm/mali: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

The CMA helper is already using the drm_fb_helper_generic_probe part of
the generic fbdev emulation. This patch makes full use of the generic
fbdev emulation by using its drm_client callbacks. This means that
drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
now handled by the emulation code. Additionally fbdev unregister happens
automatically on drm_dev_unregister().

The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
driver. This is done to highlight the fact that fbdev emulation is an
internal client that makes use of the driver, it is not part of the
driver as such. If fbdev setup fails, an error is printed, but the driver
succeeds probing.

Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Brian Starkey <brian.starkey@arm.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/arm/malidp_drv.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index 08b5bb219816..3171ffaadd77 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -260,7 +260,6 @@ static const struct drm_mode_config_helper_funcs malidp_mode_config_helpers = {
 
 static const struct drm_mode_config_funcs malidp_mode_config_funcs = {
 	.fb_create = drm_gem_fb_create,
-	.output_poll_changed = drm_fb_helper_output_poll_changed,
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = drm_atomic_helper_commit,
 };
@@ -450,7 +449,6 @@ static int malidp_debugfs_init(struct drm_minor *minor)
 static struct drm_driver malidp_driver = {
 	.driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC |
 			   DRIVER_PRIME,
-	.lastclose = drm_fb_helper_lastclose,
 	.gem_free_object_unlocked = drm_gem_cma_free_object,
 	.gem_vm_ops = &drm_gem_cma_vm_ops,
 	.dumb_create = malidp_dumb_create,
@@ -762,22 +760,18 @@ static int malidp_bind(struct device *dev)
 
 	drm_mode_config_reset(drm);
 
-	ret = drm_fb_cma_fbdev_init(drm, 32, 0);
-	if (ret)
-		goto fbdev_fail;
-
 	drm_kms_helper_poll_init(drm);
 
 	ret = drm_dev_register(drm, 0);
 	if (ret)
 		goto register_fail;
 
+	drm_fbdev_generic_setup(drm, 32);
+
 	return 0;
 
 register_fail:
-	drm_fb_cma_fbdev_fini(drm);
 	drm_kms_helper_poll_fini(drm);
-fbdev_fail:
 	pm_runtime_get_sync(dev);
 vblank_fail:
 	malidp_se_irq_fini(hwdev);
@@ -814,7 +808,6 @@ static void malidp_unbind(struct device *dev)
 	struct malidp_hw_device *hwdev = malidp->dev;
 
 	drm_dev_unregister(drm);
-	drm_fb_cma_fbdev_fini(drm);
 	drm_kms_helper_poll_fini(drm);
 	pm_runtime_get_sync(dev);
 	drm_crtc_vblank_off(&malidp->crtc);
-- 
2.15.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 09/20] drm/arm/mali: Use drm_fbdev_generic_setup()

[Liviu Dudau]

On Sat, Sep 08, 2018 at 03:46:37PM +0200, Noralf Trønnes wrote:
> The CMA helper is already using the drm_fb_helper_generic_probe part of
> the generic fbdev emulation. This patch makes full use of the generic
> fbdev emulation by using its drm_client callbacks. This means that
> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
> now handled by the emulation code. Additionally fbdev unregister happens
> automatically on drm_dev_unregister().
> 
> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
> driver. This is done to highlight the fact that fbdev emulation is an
> internal client that makes use of the driver, it is not part of the
> driver as such. If fbdev setup fails, an error is printed, but the driver
> succeeds probing.
> 
> Cc: Liviu Dudau <liviu.dudau@arm.com>

Acked-by: Liviu Dudau <liviu.dudau@arm.com>

Unless you want me to pick this patch into the mali-dp tree, I'm going
to assume that you're going to push the whole series through drm-misc-next.

Best regards,
Liviu

> Cc: Brian Starkey <brian.starkey@arm.com>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  drivers/gpu/drm/arm/malidp_drv.c | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
> index 08b5bb219816..3171ffaadd77 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.c
> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> @@ -260,7 +260,6 @@ static const struct drm_mode_config_helper_funcs malidp_mode_config_helpers = {
>  
>  static const struct drm_mode_config_funcs malidp_mode_config_funcs = {
>  	.fb_create = drm_gem_fb_create,
> -	.output_poll_changed = drm_fb_helper_output_poll_changed,
>  	.atomic_check = drm_atomic_helper_check,
>  	.atomic_commit = drm_atomic_helper_commit,
>  };
> @@ -450,7 +449,6 @@ static int malidp_debugfs_init(struct drm_minor *minor)
>  static struct drm_driver malidp_driver = {
>  	.driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC |
>  			   DRIVER_PRIME,
> -	.lastclose = drm_fb_helper_lastclose,
>  	.gem_free_object_unlocked = drm_gem_cma_free_object,
>  	.gem_vm_ops = &drm_gem_cma_vm_ops,
>  	.dumb_create = malidp_dumb_create,
> @@ -762,22 +760,18 @@ static int malidp_bind(struct device *dev)
>  
>  	drm_mode_config_reset(drm);
>  
> -	ret = drm_fb_cma_fbdev_init(drm, 32, 0);
> -	if (ret)
> -		goto fbdev_fail;
> -
>  	drm_kms_helper_poll_init(drm);
>  
>  	ret = drm_dev_register(drm, 0);
>  	if (ret)
>  		goto register_fail;
>  
> +	drm_fbdev_generic_setup(drm, 32);
> +
>  	return 0;
>  
>  register_fail:
> -	drm_fb_cma_fbdev_fini(drm);
>  	drm_kms_helper_poll_fini(drm);
> -fbdev_fail:
>  	pm_runtime_get_sync(dev);
>  vblank_fail:
>  	malidp_se_irq_fini(hwdev);
> @@ -814,7 +808,6 @@ static void malidp_unbind(struct device *dev)
>  	struct malidp_hw_device *hwdev = malidp->dev;
>  
>  	drm_dev_unregister(drm);
> -	drm_fb_cma_fbdev_fini(drm);
>  	drm_kms_helper_poll_fini(drm);
>  	pm_runtime_get_sync(dev);
>  	drm_crtc_vblank_off(&malidp->crtc);
> -- 
> 2.15.1
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 09/20] drm/arm/mali: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

Den 11.09.2018 14.18, skrev Liviu Dudau:
> On Sat, Sep 08, 2018 at 03:46:37PM +0200, Noralf Trønnes wrote:
>> The CMA helper is already using the drm_fb_helper_generic_probe part of
>> the generic fbdev emulation. This patch makes full use of the generic
>> fbdev emulation by using its drm_client callbacks. This means that
>> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
>> now handled by the emulation code. Additionally fbdev unregister happens
>> automatically on drm_dev_unregister().
>>
>> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
>> driver. This is done to highlight the fact that fbdev emulation is an
>> internal client that makes use of the driver, it is not part of the
>> driver as such. If fbdev setup fails, an error is printed, but the driver
>> succeeds probing.
>>
>> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Acked-by: Liviu Dudau <liviu.dudau@arm.com>
>
> Unless you want me to pick this patch into the mali-dp tree, I'm going
> to assume that you're going to push the whole series through drm-misc-next.

Applied to drm-misc-next, thanks.

Noralf.

> Best regards,
> Liviu
>
>> Cc: Brian Starkey <brian.starkey@arm.com>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>>   drivers/gpu/drm/arm/malidp_drv.c | 11 ++---------
>>   1 file changed, 2 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
>> index 08b5bb219816..3171ffaadd77 100644
>> --- a/drivers/gpu/drm/arm/malidp_drv.c
>> +++ b/drivers/gpu/drm/arm/malidp_drv.c
>> @@ -260,7 +260,6 @@ static const struct drm_mode_config_helper_funcs malidp_mode_config_helpers = {
>>   
>>   static const struct drm_mode_config_funcs malidp_mode_config_funcs = {
>>   	.fb_create = drm_gem_fb_create,
>> -	.output_poll_changed = drm_fb_helper_output_poll_changed,
>>   	.atomic_check = drm_atomic_helper_check,
>>   	.atomic_commit = drm_atomic_helper_commit,
>>   };
>> @@ -450,7 +449,6 @@ static int malidp_debugfs_init(struct drm_minor *minor)
>>   static struct drm_driver malidp_driver = {
>>   	.driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC |
>>   			   DRIVER_PRIME,
>> -	.lastclose = drm_fb_helper_lastclose,
>>   	.gem_free_object_unlocked = drm_gem_cma_free_object,
>>   	.gem_vm_ops = &drm_gem_cma_vm_ops,
>>   	.dumb_create = malidp_dumb_create,
>> @@ -762,22 +760,18 @@ static int malidp_bind(struct device *dev)
>>   
>>   	drm_mode_config_reset(drm);
>>   
>> -	ret = drm_fb_cma_fbdev_init(drm, 32, 0);
>> -	if (ret)
>> -		goto fbdev_fail;
>> -
>>   	drm_kms_helper_poll_init(drm);
>>   
>>   	ret = drm_dev_register(drm, 0);
>>   	if (ret)
>>   		goto register_fail;
>>   
>> +	drm_fbdev_generic_setup(drm, 32);
>> +
>>   	return 0;
>>   
>>   register_fail:
>> -	drm_fb_cma_fbdev_fini(drm);
>>   	drm_kms_helper_poll_fini(drm);
>> -fbdev_fail:
>>   	pm_runtime_get_sync(dev);
>>   vblank_fail:
>>   	malidp_se_irq_fini(hwdev);
>> @@ -814,7 +808,6 @@ static void malidp_unbind(struct device *dev)
>>   	struct malidp_hw_device *hwdev = malidp->dev;
>>   
>>   	drm_dev_unregister(drm);
>> -	drm_fb_cma_fbdev_fini(drm);
>>   	drm_kms_helper_poll_fini(drm);
>>   	pm_runtime_get_sync(dev);
>>   	drm_crtc_vblank_off(&malidp->crtc);
>> -- 
>> 2.15.1
>>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 10/20] drm/atmel-hlcdc: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

The CMA helper is already using the drm_fb_helper_generic_probe part of
the generic fbdev emulation. This patch makes full use of the generic
fbdev emulation by using its drm_client callbacks. This means that
drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
now handled by the emulation code. Additionally fbdev unregister happens
automatically on drm_dev_unregister().

The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
driver. This is done to highlight the fact that fbdev emulation is an
internal client that makes use of the driver, it is not part of the
driver as such. If fbdev setup fails, an error is printed, but the driver
succeeds probing.

Cc: Boris Brezillon <boris.brezillon@bootlin.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index 843cac222e60..5114b81f9675 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -556,7 +556,6 @@ static int atmel_hlcdc_dc_atomic_commit(struct drm_device *dev,
 
 static const struct drm_mode_config_funcs mode_config_funcs = {
 	.fb_create = atmel_hlcdc_fb_create,
-	.output_poll_changed = drm_fb_helper_output_poll_changed,
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = atmel_hlcdc_dc_atomic_commit,
 };
@@ -658,8 +657,6 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev)
 
 	platform_set_drvdata(pdev, dev);
 
-	drm_fb_cma_fbdev_init(dev, 24, 0);
-
 	drm_kms_helper_poll_init(dev);
 
 	return 0;
@@ -678,7 +675,6 @@ static void atmel_hlcdc_dc_unload(struct drm_device *dev)
 {
 	struct atmel_hlcdc_dc *dc = dev->dev_private;
 
-	drm_fb_cma_fbdev_fini(dev);
 	flush_workqueue(dc->wq);
 	drm_kms_helper_poll_fini(dev);
 	drm_atomic_helper_shutdown(dev);
@@ -727,7 +723,6 @@ static struct drm_driver atmel_hlcdc_dc_driver = {
 	.driver_features = DRIVER_HAVE_IRQ | DRIVER_GEM |
 			   DRIVER_MODESET | DRIVER_PRIME |
 			   DRIVER_ATOMIC,
-	.lastclose = drm_fb_helper_lastclose,
 	.irq_handler = atmel_hlcdc_dc_irq_handler,
 	.irq_preinstall = atmel_hlcdc_dc_irq_uninstall,
 	.irq_postinstall = atmel_hlcdc_dc_irq_postinstall,
@@ -769,6 +764,8 @@ static int atmel_hlcdc_dc_drm_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_unload;
 
+	drm_fbdev_generic_setup(ddev, 24);
+
 	return 0;
 
 err_unload:
-- 
2.15.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 11/20] drm/imx: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

The CMA helper is already using the drm_fb_helper_generic_probe part of
the generic fbdev emulation. This patch makes full use of the generic
fbdev emulation by using its drm_client callbacks. This means that
drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
now handled by the emulation code. Additionally fbdev unregister happens
automatically on drm_dev_unregister().

The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
driver. This is done to highlight the fact that fbdev emulation is an
internal client that makes use of the driver, it is not part of the
driver as such. If fbdev setup fails, an error is printed, but the driver
succeeds probing.

CONFIG_DRM_FBDEV_EMULATION wasn't honoured by the CMA helper, but it is by
drm_fb_helper.

Cc: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/imx/imx-drm-core.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c
index 5ea0c82f9957..a70f3131a377 100644
--- a/drivers/gpu/drm/imx/imx-drm-core.c
+++ b/drivers/gpu/drm/imx/imx-drm-core.c
@@ -86,7 +86,6 @@ static int imx_drm_atomic_check(struct drm_device *dev,
 
 static const struct drm_mode_config_funcs imx_drm_mode_config_funcs = {
 	.fb_create = drm_gem_fb_create,
-	.output_poll_changed = drm_fb_helper_output_poll_changed,
 	.atomic_check = imx_drm_atomic_check,
 	.atomic_commit = drm_atomic_helper_commit,
 };
@@ -165,7 +164,6 @@ static const struct drm_ioctl_desc imx_drm_ioctls[] = {
 static struct drm_driver imx_drm_driver = {
 	.driver_features	= DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME |
 				  DRIVER_ATOMIC,
-	.lastclose		= drm_fb_helper_lastclose,
 	.gem_free_object_unlocked = drm_gem_cma_free_object,
 	.gem_vm_ops		= &drm_gem_cma_vm_ops,
 	.dumb_create		= drm_gem_cma_dumb_create,
@@ -263,30 +261,23 @@ static int imx_drm_bind(struct device *dev)
 	 * The fb helper takes copies of key hardware information, so the
 	 * crtcs/connectors/encoders must not change after this point.
 	 */
-#if IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION)
 	if (legacyfb_depth != 16 && legacyfb_depth != 32) {
 		dev_warn(dev, "Invalid legacyfb_depth.  Defaulting to 16bpp\n");
 		legacyfb_depth = 16;
 	}
-	ret = drm_fb_cma_fbdev_init(drm, legacyfb_depth, MAX_CRTC);
-	if (ret)
-		goto err_unbind;
-#endif
 
 	drm_kms_helper_poll_init(drm);
 
 	ret = drm_dev_register(drm, 0);
 	if (ret)
-		goto err_fbhelper;
+		goto err_poll_fini;
+
+	drm_fbdev_generic_setup(drm, legacyfb_depth);
 
 	return 0;
 
-err_fbhelper:
+err_poll_fini:
 	drm_kms_helper_poll_fini(drm);
-#if IS_ENABLED(CONFIG_DRM_FBDEV_EMULATION)
-	drm_fb_cma_fbdev_fini(drm);
-err_unbind:
-#endif
 	component_unbind_all(drm->dev, drm);
 err_kms:
 	drm_mode_config_cleanup(drm);
@@ -303,8 +294,6 @@ static void imx_drm_unbind(struct device *dev)
 
 	drm_kms_helper_poll_fini(drm);
 
-	drm_fb_cma_fbdev_fini(drm);
-
 	drm_mode_config_cleanup(drm);
 
 	component_unbind_all(drm->dev, drm);
-- 
2.15.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 11/20] drm/imx: Use drm_fbdev_generic_setup()

[Philipp Zabel]

Hi Noralf,

On Sat, 2018-09-08 at 15:46 +0200, Noralf Trønnes wrote:
> The CMA helper is already using the drm_fb_helper_generic_probe part of
> the generic fbdev emulation. This patch makes full use of the generic
> fbdev emulation by using its drm_client callbacks. This means that
> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
> now handled by the emulation code. Additionally fbdev unregister happens
> automatically on drm_dev_unregister().
> 
> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
> driver. This is done to highlight the fact that fbdev emulation is an
> internal client that makes use of the driver, it is not part of the
> driver as such. If fbdev setup fails, an error is printed, but the driver
> succeeds probing.
> 
> CONFIG_DRM_FBDEV_EMULATION wasn't honoured by the CMA helper, but it is by
> drm_fb_helper.
> 
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
Acked-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 11/20] drm/imx: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

Den 14.09.2018 13.42, skrev Philipp Zabel:
> Hi Noralf,
>
> On Sat, 2018-09-08 at 15:46 +0200, Noralf Trønnes wrote:
>> The CMA helper is already using the drm_fb_helper_generic_probe part of
>> the generic fbdev emulation. This patch makes full use of the generic
>> fbdev emulation by using its drm_client callbacks. This means that
>> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
>> now handled by the emulation code. Additionally fbdev unregister happens
>> automatically on drm_dev_unregister().
>>
>> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
>> driver. This is done to highlight the fact that fbdev emulation is an
>> internal client that makes use of the driver, it is not part of the
>> driver as such. If fbdev setup fails, an error is printed, but the driver
>> succeeds probing.
>>
>> CONFIG_DRM_FBDEV_EMULATION wasn't honoured by the CMA helper, but it is by
>> drm_fb_helper.
>>
>> Cc: Philipp Zabel <p.zabel@pengutronix.de>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
> Acked-by: Philipp Zabel <p.zabel@pengutronix.de>

Applied to drm-misc-next, thanks.

Noralf.

> regards
> Philipp

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 12/20] drm/pl111: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

The CMA helper is already using the drm_fb_helper_generic_probe part of
the generic fbdev emulation. This patch makes full use of the generic
fbdev emulation by using its drm_client callbacks. This means that
drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
now handled by the emulation code. Additionally fbdev unregister happens
automatically on drm_dev_unregister().

The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
driver. This is done to highlight the fact that fbdev emulation is an
internal client that makes use of the driver, it is not part of the
driver as such. If fbdev setup fails, an error is printed, but the driver
succeeds probing.

Cc: Eric Anholt <eric@anholt.net>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/pl111/pl111_drv.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c
index 47fe30223444..33e0483d62ae 100644
--- a/drivers/gpu/drm/pl111/pl111_drv.c
+++ b/drivers/gpu/drm/pl111/pl111_drv.c
@@ -194,8 +194,6 @@ static int pl111_modeset_init(struct drm_device *dev)
 
 	drm_mode_config_reset(dev);
 
-	drm_fb_cma_fbdev_init(dev, priv->variant->fb_bpp, 0);
-
 	drm_kms_helper_poll_init(dev);
 
 	goto finish;
@@ -232,7 +230,6 @@ DEFINE_DRM_GEM_CMA_FOPS(drm_fops);
 static struct drm_driver pl111_drm_driver = {
 	.driver_features =
 		DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | DRIVER_ATOMIC,
-	.lastclose = drm_fb_helper_lastclose,
 	.ioctls = NULL,
 	.fops = &drm_fops,
 	.name = "pl111",
@@ -332,6 +329,8 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
 	if (ret < 0)
 		goto dev_put;
 
+	drm_fbdev_generic_setup(drm, priv->variant->fb_bpp);
+
 	return 0;
 
 dev_put:
@@ -348,7 +347,6 @@ static int pl111_amba_remove(struct amba_device *amba_dev)
 	struct pl111_drm_dev_private *priv = drm->dev_private;
 
 	drm_dev_unregister(drm);
-	drm_fb_cma_fbdev_fini(drm);
 	if (priv->panel)
 		drm_panel_bridge_remove(priv->bridge);
 	drm_mode_config_cleanup(drm);
-- 
2.15.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 12/20] drm/pl111: Use drm_fbdev_generic_setup()

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 503 bytes --]

Noralf Trønnes <noralf@tronnes.org> writes:

> The CMA helper is already using the drm_fb_helper_generic_probe part of
> the generic fbdev emulation. This patch makes full use of the generic
> fbdev emulation by using its drm_client callbacks. This means that
> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
> now handled by the emulation code. Additionally fbdev unregister happens
> automatically on drm_dev_unregister().

Acked-by: Eric Anholt <eric@anholt.net>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 12/20] drm/pl111: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

Den 13.09.2018 02.07, skrev Eric Anholt:
> Noralf Trønnes <noralf@tronnes.org> writes:
>
>> The CMA helper is already using the drm_fb_helper_generic_probe part of
>> the generic fbdev emulation. This patch makes full use of the generic
>> fbdev emulation by using its drm_client callbacks. This means that
>> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
>> now handled by the emulation code. Additionally fbdev unregister happens
>> automatically on drm_dev_unregister().
> Acked-by: Eric Anholt <eric@anholt.net>

Applied to drm-misc-next, thanks.

Noralf

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 13/20] drm/sti: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

The CMA helper is already using the drm_fb_helper_generic_probe part of
the generic fbdev emulation. This patch makes full use of the generic
fbdev emulation by using its drm_client callbacks. This means that
drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
now handled by the emulation code. Additionally fbdev unregister happens
automatically on drm_dev_unregister().

If drm_fbdev_generic_setup() fails, an error is printed by the function.

drm_fbdev_generic_setup() handles mode_config.num_connector being zero.
In that case it retries fbdev setup on the next .output_poll_changed.

Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Cc: Vincent Abriou <vincent.abriou@st.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/sti/sti_drv.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
index 832fc43960ee..6dced8abcf16 100644
--- a/drivers/gpu/drm/sti/sti_drv.c
+++ b/drivers/gpu/drm/sti/sti_drv.c
@@ -121,7 +121,6 @@ static int sti_drm_dbg_init(struct drm_minor *minor)
 
 static const struct drm_mode_config_funcs sti_mode_config_funcs = {
 	.fb_create = drm_gem_fb_create,
-	.output_poll_changed = drm_fb_helper_output_poll_changed,
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = drm_atomic_helper_commit,
 };
@@ -206,7 +205,6 @@ static void sti_cleanup(struct drm_device *ddev)
 {
 	struct sti_private *private = ddev->dev_private;
 
-	drm_fb_cma_fbdev_fini(ddev);
 	drm_kms_helper_poll_fini(ddev);
 	component_unbind_all(ddev->dev, ddev);
 	kfree(private);
@@ -236,11 +234,7 @@ static int sti_bind(struct device *dev)
 
 	drm_mode_config_reset(ddev);
 
-	if (ddev->mode_config.num_connector) {
-		ret = drm_fb_cma_fbdev_init(ddev, 32, 0);
-		if (ret)
-			DRM_DEBUG_DRIVER("Warning: fails to create fbdev\n");
-	}
+	drm_fbdev_generic_setup(ddev, 32);
 
 	return 0;
 
-- 
2.15.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 13/20] drm/sti: Use drm_fbdev_generic_setup()

[Benjamin Gaignard]

2018-09-08 15:46 GMT+02:00 Noralf Trønnes <noralf@tronnes.org>:
> The CMA helper is already using the drm_fb_helper_generic_probe part of
> the generic fbdev emulation. This patch makes full use of the generic
> fbdev emulation by using its drm_client callbacks. This means that
> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
> now handled by the emulation code. Additionally fbdev unregister happens
> automatically on drm_dev_unregister().
>
> If drm_fbdev_generic_setup() fails, an error is printed by the function.
>
> drm_fbdev_generic_setup() handles mode_config.num_connector being zero.
> In that case it retries fbdev setup on the next .output_poll_changed.
>
> Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> Cc: Vincent Abriou <vincent.abriou@st.com>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

Acked-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>

> ---
>  drivers/gpu/drm/sti/sti_drv.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
> index 832fc43960ee..6dced8abcf16 100644
> --- a/drivers/gpu/drm/sti/sti_drv.c
> +++ b/drivers/gpu/drm/sti/sti_drv.c
> @@ -121,7 +121,6 @@ static int sti_drm_dbg_init(struct drm_minor *minor)
>
>  static const struct drm_mode_config_funcs sti_mode_config_funcs = {
>         .fb_create = drm_gem_fb_create,
> -       .output_poll_changed = drm_fb_helper_output_poll_changed,
>         .atomic_check = drm_atomic_helper_check,
>         .atomic_commit = drm_atomic_helper_commit,
>  };
> @@ -206,7 +205,6 @@ static void sti_cleanup(struct drm_device *ddev)
>  {
>         struct sti_private *private = ddev->dev_private;
>
> -       drm_fb_cma_fbdev_fini(ddev);
>         drm_kms_helper_poll_fini(ddev);
>         component_unbind_all(ddev->dev, ddev);
>         kfree(private);
> @@ -236,11 +234,7 @@ static int sti_bind(struct device *dev)
>
>         drm_mode_config_reset(ddev);
>
> -       if (ddev->mode_config.num_connector) {
> -               ret = drm_fb_cma_fbdev_init(ddev, 32, 0);
> -               if (ret)
> -                       DRM_DEBUG_DRIVER("Warning: fails to create fbdev\n");
> -       }
> +       drm_fbdev_generic_setup(ddev, 32);
>
>         return 0;
>
> --
> 2.15.1
>



-- 
Benjamin Gaignard

Graphic Study Group

Linaro.org │ Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 13/20] drm/sti: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

Den 10.09.2018 11.39, skrev Benjamin Gaignard:
> 2018-09-08 15:46 GMT+02:00 Noralf Trønnes <noralf@tronnes.org>:
>> The CMA helper is already using the drm_fb_helper_generic_probe part of
>> the generic fbdev emulation. This patch makes full use of the generic
>> fbdev emulation by using its drm_client callbacks. This means that
>> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
>> now handled by the emulation code. Additionally fbdev unregister happens
>> automatically on drm_dev_unregister().
>>
>> If drm_fbdev_generic_setup() fails, an error is printed by the function.
>>
>> drm_fbdev_generic_setup() handles mode_config.num_connector being zero.
>> In that case it retries fbdev setup on the next .output_poll_changed.
>>
>> Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
>> Cc: Vincent Abriou <vincent.abriou@st.com>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> Acked-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>

Applied to drm-misc-next, thanks.

Noralf

>> ---
>>   drivers/gpu/drm/sti/sti_drv.c | 8 +-------
>>   1 file changed, 1 insertion(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
>> index 832fc43960ee..6dced8abcf16 100644
>> --- a/drivers/gpu/drm/sti/sti_drv.c
>> +++ b/drivers/gpu/drm/sti/sti_drv.c
>> @@ -121,7 +121,6 @@ static int sti_drm_dbg_init(struct drm_minor *minor)
>>
>>   static const struct drm_mode_config_funcs sti_mode_config_funcs = {
>>          .fb_create = drm_gem_fb_create,
>> -       .output_poll_changed = drm_fb_helper_output_poll_changed,
>>          .atomic_check = drm_atomic_helper_check,
>>          .atomic_commit = drm_atomic_helper_commit,
>>   };
>> @@ -206,7 +205,6 @@ static void sti_cleanup(struct drm_device *ddev)
>>   {
>>          struct sti_private *private = ddev->dev_private;
>>
>> -       drm_fb_cma_fbdev_fini(ddev);
>>          drm_kms_helper_poll_fini(ddev);
>>          component_unbind_all(ddev->dev, ddev);
>>          kfree(private);
>> @@ -236,11 +234,7 @@ static int sti_bind(struct device *dev)
>>
>>          drm_mode_config_reset(ddev);
>>
>> -       if (ddev->mode_config.num_connector) {
>> -               ret = drm_fb_cma_fbdev_init(ddev, 32, 0);
>> -               if (ret)
>> -                       DRM_DEBUG_DRIVER("Warning: fails to create fbdev\n");
>> -       }
>> +       drm_fbdev_generic_setup(ddev, 32);
>>
>>          return 0;
>>
>> --
>> 2.15.1
>>
>
>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 14/20] drm/stm: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

The CMA helper is already using the drm_fb_helper_generic_probe part of
the generic fbdev emulation. This patch makes full use of the generic
fbdev emulation by using its drm_client callbacks. This means that
drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
now handled by the emulation code. Additionally fbdev unregister happens
automatically on drm_dev_unregister().

The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
driver. This is done to highlight the fact that fbdev emulation is an
internal client that makes use of the driver, it is not part of the
driver as such. If fbdev setup fails, an error is printed, but the driver
succeeds probing.

drm_fbdev_generic_setup() handles mode_config.num_connector being zero.
In that case it retries fbdev setup on the next .output_poll_changed.

Cc: Yannick Fertre <yannick.fertre@st.com>
Cc: Philippe Cornu <philippe.cornu@st.com>
Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Cc: Vincent Abriou <vincent.abriou@st.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/stm/drv.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/stm/drv.c b/drivers/gpu/drm/stm/drv.c
index f2021b23554d..97eee8660014 100644
--- a/drivers/gpu/drm/stm/drv.c
+++ b/drivers/gpu/drm/stm/drv.c
@@ -26,7 +26,6 @@
 
 static const struct drm_mode_config_funcs drv_mode_config_funcs = {
 	.fb_create = drm_gem_fb_create,
-	.output_poll_changed = drm_fb_helper_output_poll_changed,
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = drm_atomic_helper_commit,
 };
@@ -52,7 +51,6 @@ DEFINE_DRM_GEM_CMA_FOPS(drv_driver_fops);
 static struct drm_driver drv_driver = {
 	.driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME |
 			   DRIVER_ATOMIC,
-	.lastclose = drm_fb_helper_lastclose,
 	.name = "stm",
 	.desc = "STMicroelectronics SoC DRM",
 	.date = "20170330",
@@ -108,12 +106,6 @@ static int drv_load(struct drm_device *ddev)
 	drm_mode_config_reset(ddev);
 	drm_kms_helper_poll_init(ddev);
 
-	if (ddev->mode_config.num_connector) {
-		ret = drm_fb_cma_fbdev_init(ddev, 16, 0);
-		if (ret)
-			DRM_DEBUG("Warning: fails to create fbdev\n");
-	}
-
 	platform_set_drvdata(pdev, ddev);
 
 	return 0;
@@ -126,7 +118,6 @@ static void drv_unload(struct drm_device *ddev)
 {
 	DRM_DEBUG("%s\n", __func__);
 
-	drm_fb_cma_fbdev_fini(ddev);
 	drm_kms_helper_poll_fini(ddev);
 	ltdc_unload(ddev);
 	drm_mode_config_cleanup(ddev);
@@ -154,6 +145,8 @@ static int stm_drm_platform_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_put;
 
+	drm_fbdev_generic_setup(ddev, 16);
+
 	return 0;
 
 err_put:
-- 
2.15.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 14/20] drm/stm: Use drm_fbdev_generic_setup()

[Yannick FERTRE]

Hi Noralf,
many thanks for your patch.

Acked-by: Yannick Fertré <yannick.fertre@st.com>


On 09/08/2018 03:46 PM, Noralf Trønnes wrote:
> The CMA helper is already using the drm_fb_helper_generic_probe part of
> the generic fbdev emulation. This patch makes full use of the generic
> fbdev emulation by using its drm_client callbacks. This means that
> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
> now handled by the emulation code. Additionally fbdev unregister happens
> automatically on drm_dev_unregister().
>
> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
> driver. This is done to highlight the fact that fbdev emulation is an
> internal client that makes use of the driver, it is not part of the
> driver as such. If fbdev setup fails, an error is printed, but the driver
> succeeds probing.
>
> drm_fbdev_generic_setup() handles mode_config.num_connector being zero.
> In that case it retries fbdev setup on the next .output_poll_changed.
>
> Cc: Yannick Fertre <yannick.fertre@st.com>
> Cc: Philippe Cornu <philippe.cornu@st.com>
> Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> Cc: Vincent Abriou <vincent.abriou@st.com>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>   drivers/gpu/drm/stm/drv.c | 11 ++---------
>   1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/stm/drv.c b/drivers/gpu/drm/stm/drv.c
> index f2021b23554d..97eee8660014 100644
> --- a/drivers/gpu/drm/stm/drv.c
> +++ b/drivers/gpu/drm/stm/drv.c
> @@ -26,7 +26,6 @@
>   
>   static const struct drm_mode_config_funcs drv_mode_config_funcs = {
>   	.fb_create = drm_gem_fb_create,
> -	.output_poll_changed = drm_fb_helper_output_poll_changed,
>   	.atomic_check = drm_atomic_helper_check,
>   	.atomic_commit = drm_atomic_helper_commit,
>   };
> @@ -52,7 +51,6 @@ DEFINE_DRM_GEM_CMA_FOPS(drv_driver_fops);
>   static struct drm_driver drv_driver = {
>   	.driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME |
>   			   DRIVER_ATOMIC,
> -	.lastclose = drm_fb_helper_lastclose,
>   	.name = "stm",
>   	.desc = "STMicroelectronics SoC DRM",
>   	.date = "20170330",
> @@ -108,12 +106,6 @@ static int drv_load(struct drm_device *ddev)
>   	drm_mode_config_reset(ddev);
>   	drm_kms_helper_poll_init(ddev);
>   
> -	if (ddev->mode_config.num_connector) {
> -		ret = drm_fb_cma_fbdev_init(ddev, 16, 0);
> -		if (ret)
> -			DRM_DEBUG("Warning: fails to create fbdev\n");
> -	}
> -
>   	platform_set_drvdata(pdev, ddev);
>   
>   	return 0;
> @@ -126,7 +118,6 @@ static void drv_unload(struct drm_device *ddev)
>   {
>   	DRM_DEBUG("%s\n", __func__);
>   
> -	drm_fb_cma_fbdev_fini(ddev);
>   	drm_kms_helper_poll_fini(ddev);
>   	ltdc_unload(ddev);
>   	drm_mode_config_cleanup(ddev);
> @@ -154,6 +145,8 @@ static int stm_drm_platform_probe(struct platform_device *pdev)
>   	if (ret)
>   		goto err_put;
>   
> +	drm_fbdev_generic_setup(ddev, 16);
> +
>   	return 0;
>   
>   err_put:

-- 
Yannick Fertré | TINA: 166 7152 | Tel: +33 244027152 | Mobile: +33 620600270
Microcontrollers and Digital ICs Group | Microcontrolleurs Division
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 14/20] drm/stm: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

Den 27.09.2018 13.45, skrev Yannick FERTRE:
> Hi Noralf,
> many thanks for your patch.
>
> Acked-by: Yannick Fertré <yannick.fertre@st.com>

Applied to drm-misc-next, thanks.

Noralf.

>
> On 09/08/2018 03:46 PM, Noralf Trønnes wrote:
>> The CMA helper is already using the drm_fb_helper_generic_probe part of
>> the generic fbdev emulation. This patch makes full use of the generic
>> fbdev emulation by using its drm_client callbacks. This means that
>> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
>> now handled by the emulation code. Additionally fbdev unregister happens
>> automatically on drm_dev_unregister().
>>
>> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
>> driver. This is done to highlight the fact that fbdev emulation is an
>> internal client that makes use of the driver, it is not part of the
>> driver as such. If fbdev setup fails, an error is printed, but the driver
>> succeeds probing.
>>
>> drm_fbdev_generic_setup() handles mode_config.num_connector being zero.
>> In that case it retries fbdev setup on the next .output_poll_changed.
>>
>> Cc: Yannick Fertre <yannick.fertre@st.com>
>> Cc: Philippe Cornu <philippe.cornu@st.com>
>> Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
>> Cc: Vincent Abriou <vincent.abriou@st.com>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>>    drivers/gpu/drm/stm/drv.c | 11 ++---------
>>    1 file changed, 2 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/stm/drv.c b/drivers/gpu/drm/stm/drv.c
>> index f2021b23554d..97eee8660014 100644
>> --- a/drivers/gpu/drm/stm/drv.c
>> +++ b/drivers/gpu/drm/stm/drv.c
>> @@ -26,7 +26,6 @@
>>    
>>    static const struct drm_mode_config_funcs drv_mode_config_funcs = {
>>    	.fb_create = drm_gem_fb_create,
>> -	.output_poll_changed = drm_fb_helper_output_poll_changed,
>>    	.atomic_check = drm_atomic_helper_check,
>>    	.atomic_commit = drm_atomic_helper_commit,
>>    };
>> @@ -52,7 +51,6 @@ DEFINE_DRM_GEM_CMA_FOPS(drv_driver_fops);
>>    static struct drm_driver drv_driver = {
>>    	.driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME |
>>    			   DRIVER_ATOMIC,
>> -	.lastclose = drm_fb_helper_lastclose,
>>    	.name = "stm",
>>    	.desc = "STMicroelectronics SoC DRM",
>>    	.date = "20170330",
>> @@ -108,12 +106,6 @@ static int drv_load(struct drm_device *ddev)
>>    	drm_mode_config_reset(ddev);
>>    	drm_kms_helper_poll_init(ddev);
>>    
>> -	if (ddev->mode_config.num_connector) {
>> -		ret = drm_fb_cma_fbdev_init(ddev, 16, 0);
>> -		if (ret)
>> -			DRM_DEBUG("Warning: fails to create fbdev\n");
>> -	}
>> -
>>    	platform_set_drvdata(pdev, ddev);
>>    
>>    	return 0;
>> @@ -126,7 +118,6 @@ static void drv_unload(struct drm_device *ddev)
>>    {
>>    	DRM_DEBUG("%s\n", __func__);
>>    
>> -	drm_fb_cma_fbdev_fini(ddev);
>>    	drm_kms_helper_poll_fini(ddev);
>>    	ltdc_unload(ddev);
>>    	drm_mode_config_cleanup(ddev);
>> @@ -154,6 +145,8 @@ static int stm_drm_platform_probe(struct platform_device *pdev)
>>    	if (ret)
>>    		goto err_put;
>>    
>> +	drm_fbdev_generic_setup(ddev, 16);
>> +
>>    	return 0;
>>    
>>    err_put:

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 15/20] drm/sun4i: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

The CMA helper is already using the drm_fb_helper_generic_probe part of
the generic fbdev emulation. This patch makes full use of the generic
fbdev emulation by using its drm_client callbacks. This means that
drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
now handled by the emulation code. Additionally fbdev unregister happens
automatically on drm_dev_unregister().

The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
driver. This is done to highlight the fact that fbdev emulation is an
internal client that makes use of the driver, it is not part of the
driver as such. If fbdev setup fails, an error is printed, but the driver
succeeds probing.

Cc: Maxime Ripard <maxime.ripard@bootlin.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/sun4i/sun4i_drv.c         | 12 +++---------
 drivers/gpu/drm/sun4i/sun4i_framebuffer.c | 12 +-----------
 drivers/gpu/drm/sun4i/sun4i_framebuffer.h |  3 +--
 3 files changed, 5 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
index 1e41c3f5fd6d..ef773d36baf0 100644
--- a/drivers/gpu/drm/sun4i/sun4i_drv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
@@ -34,7 +34,6 @@ static struct drm_driver sun4i_drv_driver = {
 	.driver_features	= DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME | DRIVER_ATOMIC,
 
 	/* Generic Operations */
-	.lastclose		= drm_fb_helper_lastclose,
 	.fops			= &sun4i_drv_fops,
 	.name			= "sun4i-drm",
 	.desc			= "Allwinner sun4i Display Engine",
@@ -105,12 +104,7 @@ static int sun4i_drv_bind(struct device *dev)
 	/* Remove early framebuffers (ie. simplefb) */
 	drm_fb_helper_remove_conflicting_framebuffers(NULL, "sun4i-drm-fb", false);
 
-	/* Create our framebuffer */
-	ret = sun4i_framebuffer_init(drm);
-	if (ret) {
-		dev_err(drm->dev, "Couldn't create our framebuffer\n");
-		goto cleanup_mode_config;
-	}
+	sun4i_framebuffer_init(drm);
 
 	/* Enable connectors polling */
 	drm_kms_helper_poll_init(drm);
@@ -119,11 +113,12 @@ static int sun4i_drv_bind(struct device *dev)
 	if (ret)
 		goto finish_poll;
 
+	drm_fbdev_generic_setup(drm, 32);
+
 	return 0;
 
 finish_poll:
 	drm_kms_helper_poll_fini(drm);
-	sun4i_framebuffer_free(drm);
 cleanup_mode_config:
 	drm_mode_config_cleanup(drm);
 	of_reserved_mem_device_release(dev);
@@ -138,7 +133,6 @@ static void sun4i_drv_unbind(struct device *dev)
 
 	drm_dev_unregister(drm);
 	drm_kms_helper_poll_fini(drm);
-	sun4i_framebuffer_free(drm);
 	drm_mode_config_cleanup(drm);
 	of_reserved_mem_device_release(dev);
 	drm_dev_put(drm);
diff --git a/drivers/gpu/drm/sun4i/sun4i_framebuffer.c b/drivers/gpu/drm/sun4i/sun4i_framebuffer.c
index 5f29850ef8ac..cb828028ae06 100644
--- a/drivers/gpu/drm/sun4i/sun4i_framebuffer.c
+++ b/drivers/gpu/drm/sun4i/sun4i_framebuffer.c
@@ -12,8 +12,6 @@
 
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
-#include <drm/drm_fb_helper.h>
-#include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
 #include <drm/drmP.h>
 
@@ -37,7 +35,6 @@ static int sun4i_de_atomic_check(struct drm_device *dev,
 }
 
 static const struct drm_mode_config_funcs sun4i_de_mode_config_funcs = {
-	.output_poll_changed	= drm_fb_helper_output_poll_changed,
 	.atomic_check		= sun4i_de_atomic_check,
 	.atomic_commit		= drm_atomic_helper_commit,
 	.fb_create		= drm_gem_fb_create,
@@ -47,7 +44,7 @@ static struct drm_mode_config_helper_funcs sun4i_de_mode_config_helpers = {
 	.atomic_commit_tail	= drm_atomic_helper_commit_tail_rpm,
 };
 
-int sun4i_framebuffer_init(struct drm_device *drm)
+void sun4i_framebuffer_init(struct drm_device *drm)
 {
 	drm_mode_config_reset(drm);
 
@@ -56,11 +53,4 @@ int sun4i_framebuffer_init(struct drm_device *drm)
 
 	drm->mode_config.funcs = &sun4i_de_mode_config_funcs;
 	drm->mode_config.helper_private = &sun4i_de_mode_config_helpers;
-
-	return drm_fb_cma_fbdev_init(drm, 32, 0);
-}
-
-void sun4i_framebuffer_free(struct drm_device *drm)
-{
-	drm_fb_cma_fbdev_fini(drm);
 }
diff --git a/drivers/gpu/drm/sun4i/sun4i_framebuffer.h b/drivers/gpu/drm/sun4i/sun4i_framebuffer.h
index 7ef0aed8384c..6fe5bd8c4026 100644
--- a/drivers/gpu/drm/sun4i/sun4i_framebuffer.h
+++ b/drivers/gpu/drm/sun4i/sun4i_framebuffer.h
@@ -13,7 +13,6 @@
 #ifndef _SUN4I_FRAMEBUFFER_H_
 #define _SUN4I_FRAMEBUFFER_H_
 
-int sun4i_framebuffer_init(struct drm_device *drm);
-void sun4i_framebuffer_free(struct drm_device *drm);
+void sun4i_framebuffer_init(struct drm_device *drm);
 
 #endif /* _SUN4I_FRAMEBUFFER_H_ */
-- 
2.15.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 15/20] drm/sun4i: Use drm_fbdev_generic_setup()

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 1113 bytes --]

On Sat, Sep 08, 2018 at 03:46:43PM +0200, Noralf Trønnes wrote:
> The CMA helper is already using the drm_fb_helper_generic_probe part of
> the generic fbdev emulation. This patch makes full use of the generic
> fbdev emulation by using its drm_client callbacks. This means that
> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
> now handled by the emulation code. Additionally fbdev unregister happens
> automatically on drm_dev_unregister().
> 
> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
> driver. This is done to highlight the fact that fbdev emulation is an
> internal client that makes use of the driver, it is not part of the
> driver as such. If fbdev setup fails, an error is printed, but the driver
> succeeds probing.
> 
> Cc: Maxime Ripard <maxime.ripard@bootlin.com>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

It's hard to tell anything about this patch without context, but is
the mali userspace still broken?

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 15/20] drm/sun4i: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

Den 10.09.2018 09.28, skrev Maxime Ripard:
> On Sat, Sep 08, 2018 at 03:46:43PM +0200, Noralf Trønnes wrote:
>> The CMA helper is already using the drm_fb_helper_generic_probe part of
>> the generic fbdev emulation. This patch makes full use of the generic
>> fbdev emulation by using its drm_client callbacks. This means that
>> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
>> now handled by the emulation code. Additionally fbdev unregister happens
>> automatically on drm_dev_unregister().
>>
>> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
>> driver. This is done to highlight the fact that fbdev emulation is an
>> internal client that makes use of the driver, it is not part of the
>> driver as such. If fbdev setup fails, an error is printed, but the driver
>> succeeds probing.
>>
>> Cc: Maxime Ripard <maxime.ripard@bootlin.com>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> It's hard to tell anything about this patch without context, but is
> the mali userspace still broken?

I replied to the cover letter with more context.

If that's the smem_start  thing, then yes.
This patch will block smem_start from being used by any DRM driver:
drm/fb: Stop leaking physical address
https://patchwork.freedesktop.org/patch/245514/

Noralf.

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 16/20] drm/tilcdc: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

The CMA helper is already using the drm_fb_helper_generic_probe part of
the generic fbdev emulation. This patch makes full use of the generic
fbdev emulation by using its drm_client callbacks. This means that
drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
now handled by the emulation code. Additionally fbdev unregister happens
automatically on drm_dev_unregister().

The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
driver. This is done to highlight the fact that fbdev emulation is an
internal client that makes use of the driver, it is not part of the
driver as such. If fbdev setup fails, an error is printed, but the driver
succeeds probing.

Cc: Jyri Sarha <jsarha@ti.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/tilcdc/tilcdc_drv.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 0fb300d41a09..94d201af3994 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -140,7 +140,6 @@ static int tilcdc_commit(struct drm_device *dev,
 
 static const struct drm_mode_config_funcs mode_config_funcs = {
 	.fb_create = tilcdc_fb_create,
-	.output_poll_changed = drm_fb_helper_output_poll_changed,
 	.atomic_check = tilcdc_atomic_check,
 	.atomic_commit = tilcdc_commit,
 };
@@ -191,9 +190,6 @@ static void tilcdc_fini(struct drm_device *dev)
 		drm_dev_unregister(dev);
 
 	drm_kms_helper_poll_fini(dev);
-
-	drm_fb_cma_fbdev_fini(dev);
-
 	drm_irq_uninstall(dev);
 	drm_mode_config_cleanup(dev);
 	tilcdc_remove_external_device(dev);
@@ -396,16 +392,14 @@ static int tilcdc_init(struct drm_driver *ddrv, struct device *dev)
 
 	drm_mode_config_reset(ddev);
 
-	ret = drm_fb_cma_fbdev_init(ddev, bpp, 0);
-	if (ret)
-		goto init_failed;
-
 	drm_kms_helper_poll_init(ddev);
 
 	ret = drm_dev_register(ddev, 0);
 	if (ret)
 		goto init_failed;
 
+	drm_fbdev_generic_setup(ddev, bpp);
+
 	priv->is_registered = true;
 	return 0;
 
@@ -519,7 +513,6 @@ DEFINE_DRM_GEM_CMA_FOPS(fops);
 static struct drm_driver tilcdc_driver = {
 	.driver_features    = (DRIVER_HAVE_IRQ | DRIVER_GEM | DRIVER_MODESET |
 			       DRIVER_PRIME | DRIVER_ATOMIC),
-	.lastclose          = drm_fb_helper_lastclose,
 	.irq_handler        = tilcdc_irq,
 	.gem_free_object_unlocked = drm_gem_cma_free_object,
 	.gem_print_info     = drm_gem_cma_print_info,
-- 
2.15.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 17/20] drm/tve200: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

The CMA helper is already using the drm_fb_helper_generic_probe part of
the generic fbdev emulation. This patch makes full use of the generic
fbdev emulation by using its drm_client callbacks. This means that
drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
now handled by the emulation code. Additionally fbdev unregister happens
automatically on drm_dev_unregister().

The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
driver. This is done to highlight the fact that fbdev emulation is an
internal client that makes use of the driver, it is not part of the
driver as such. If fbdev setup fails, an error is printed, but the driver
succeeds probing.

Cc: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/tve200/tve200_drv.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/tve200/tve200_drv.c b/drivers/gpu/drm/tve200/tve200_drv.c
index ac344ddb23bc..72efcecb44f7 100644
--- a/drivers/gpu/drm/tve200/tve200_drv.c
+++ b/drivers/gpu/drm/tve200/tve200_drv.c
@@ -126,12 +126,6 @@ static int tve200_modeset_init(struct drm_device *dev)
 	}
 
 	drm_mode_config_reset(dev);
-
-	/*
-	 * Passing in 16 here will make the RGB656 mode the default
-	 * Passing in 32 will use XRGB8888 mode
-	 */
-	drm_fb_cma_fbdev_init(dev, 16, 0);
 	drm_kms_helper_poll_init(dev);
 
 	goto finish;
@@ -149,7 +143,6 @@ DEFINE_DRM_GEM_CMA_FOPS(drm_fops);
 static struct drm_driver tve200_drm_driver = {
 	.driver_features =
 		DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME | DRIVER_ATOMIC,
-	.lastclose = drm_fb_helper_lastclose,
 	.ioctls = NULL,
 	.fops = &drm_fops,
 	.name = "tve200",
@@ -245,6 +238,12 @@ static int tve200_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto clk_disable;
 
+	/*
+	 * Passing in 16 here will make the RGB565 mode the default
+	 * Passing in 32 will use XRGB8888 mode
+	 */
+	drm_fbdev_generic_setup(drm, 16);
+
 	return 0;
 
 clk_disable:
@@ -260,7 +259,6 @@ static int tve200_remove(struct platform_device *pdev)
 	struct tve200_drm_dev_private *priv = drm->dev_private;
 
 	drm_dev_unregister(drm);
-	drm_fb_cma_fbdev_fini(drm);
 	if (priv->panel)
 		drm_panel_bridge_remove(priv->bridge);
 	drm_mode_config_cleanup(drm);
-- 
2.15.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 17/20] drm/tve200: Use drm_fbdev_generic_setup()

[Linus Walleij]

On Sat, Sep 8, 2018 at 6:53 AM Noralf Trønnes <noralf@tronnes.org> wrote:

> The CMA helper is already using the drm_fb_helper_generic_probe part of
> the generic fbdev emulation. This patch makes full use of the generic
> fbdev emulation by using its drm_client callbacks. This means that
> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
> now handled by the emulation code. Additionally fbdev unregister happens
> automatically on drm_dev_unregister().
>
> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
> driver. This is done to highlight the fact that fbdev emulation is an
> internal client that makes use of the driver, it is not part of the
> driver as such. If fbdev setup fails, an error is printed, but the driver
> succeeds probing.
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 17/20] drm/tve200: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

Den 19.09.2018 00.10, skrev Linus Walleij:
> On Sat, Sep 8, 2018 at 6:53 AM Noralf Trønnes <noralf@tronnes.org> wrote:
>
>> The CMA helper is already using the drm_fb_helper_generic_probe part of
>> the generic fbdev emulation. This patch makes full use of the generic
>> fbdev emulation by using its drm_client callbacks. This means that
>> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
>> now handled by the emulation code. Additionally fbdev unregister happens
>> automatically on drm_dev_unregister().
>>
>> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
>> driver. This is done to highlight the fact that fbdev emulation is an
>> internal client that makes use of the driver, it is not part of the
>> driver as such. If fbdev setup fails, an error is printed, but the driver
>> succeeds probing.
>>
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>

Applied to drm-misc-next, thanks.

Noralf

> Yours,
> Linus Walleij
>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 18/20] drm/vc4: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

The CMA helper is already using the drm_fb_helper_generic_probe part of
the generic fbdev emulation. This patch makes full use of the generic
fbdev emulation by using its drm_client callbacks. This means that
drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
now handled by the emulation code. Additionally fbdev unregister happens
automatically on drm_dev_unregister().

The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
driver. This is done to highlight the fact that fbdev emulation is an
internal client that makes use of the driver, it is not part of the
driver as such. If fbdev setup fails, an error is printed, but the driver
succeeds probing.

drm_fbdev_generic_setup() handles mode_config.num_connector being zero.
In that case it retries fbdev setup on the next .output_poll_changed.

Cc: Eric Anholt <eric@anholt.net>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/vc4/vc4_drv.c | 5 ++---
 drivers/gpu/drm/vc4/vc4_kms.c | 6 ------
 2 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index e2a15c63a81f..1f1780ccdbdf 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -178,7 +178,6 @@ static struct drm_driver vc4_drm_driver = {
 			    DRIVER_RENDER |
 			    DRIVER_PRIME |
 			    DRIVER_SYNCOBJ),
-	.lastclose = drm_fb_helper_lastclose,
 	.open = vc4_open,
 	.postclose = vc4_close,
 	.irq_handler = vc4_irq,
@@ -288,6 +287,8 @@ static int vc4_drm_bind(struct device *dev)
 
 	vc4_kms_load(drm);
 
+	drm_fbdev_generic_setup(drm, 32);
+
 	return 0;
 
 unbind_all:
@@ -307,8 +308,6 @@ static void vc4_drm_unbind(struct device *dev)
 
 	drm_dev_unregister(drm);
 
-	drm_fb_cma_fbdev_fini(drm);
-
 	drm_mode_config_cleanup(drm);
 
 	drm_atomic_private_obj_fini(&vc4->ctm_manager);
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index ca5aa7fba769..127468785f74 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -19,8 +19,6 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_plane_helper.h>
-#include <drm/drm_fb_helper.h>
-#include <drm/drm_fb_cma_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
 #include "vc4_drv.h"
 #include "vc4_regs.h"
@@ -394,7 +392,6 @@ vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
 }
 
 static const struct drm_mode_config_funcs vc4_mode_funcs = {
-	.output_poll_changed = drm_fb_helper_output_poll_changed,
 	.atomic_check = vc4_atomic_check,
 	.atomic_commit = vc4_atomic_commit,
 	.fb_create = vc4_fb_create,
@@ -434,9 +431,6 @@ int vc4_kms_load(struct drm_device *dev)
 
 	drm_mode_config_reset(dev);
 
-	if (dev->mode_config.num_connector)
-		drm_fb_cma_fbdev_init(dev, 32, 0);
-
 	drm_kms_helper_poll_init(dev);
 
 	return 0;
-- 
2.15.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 18/20] drm/vc4: Use drm_fbdev_generic_setup()

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 503 bytes --]

Noralf Trønnes <noralf@tronnes.org> writes:

> The CMA helper is already using the drm_fb_helper_generic_probe part of
> the generic fbdev emulation. This patch makes full use of the generic
> fbdev emulation by using its drm_client callbacks. This means that
> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
> now handled by the emulation code. Additionally fbdev unregister happens
> automatically on drm_dev_unregister().

Acked-by: Eric Anholt <eric@anholt.net>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 18/20] drm/vc4: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

Den 13.09.2018 02.06, skrev Eric Anholt:
> Noralf Trønnes <noralf@tronnes.org> writes:
>
>> The CMA helper is already using the drm_fb_helper_generic_probe part of
>> the generic fbdev emulation. This patch makes full use of the generic
>> fbdev emulation by using its drm_client callbacks. This means that
>> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
>> now handled by the emulation code. Additionally fbdev unregister happens
>> automatically on drm_dev_unregister().
> Acked-by: Eric Anholt <eric@anholt.net>

Applied to drm-misc-next, thanks.

Noralf

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 19/20] drm/zte: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

The CMA helper is already using the drm_fb_helper_generic_probe part of
the generic fbdev emulation. This patch makes full use of the generic
fbdev emulation by using its drm_client callbacks. This means that
drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
now handled by the emulation code. Additionally fbdev unregister happens
automatically on drm_dev_unregister().

The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
driver. This is done to highlight the fact that fbdev emulation is an
internal client that makes use of the driver, it is not part of the
driver as such. If fbdev setup fails, an error is printed, but the driver
succeeds probing.

Cc: Shawn Guo <shawnguo@kernel.org>
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/zte/zx_drm_drv.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/zte/zx_drm_drv.c b/drivers/gpu/drm/zte/zx_drm_drv.c
index 6f4205e80378..a50b140faf7a 100644
--- a/drivers/gpu/drm/zte/zx_drm_drv.c
+++ b/drivers/gpu/drm/zte/zx_drm_drv.c
@@ -31,7 +31,6 @@
 
 static const struct drm_mode_config_funcs zx_drm_mode_config_funcs = {
 	.fb_create = drm_gem_fb_create,
-	.output_poll_changed = drm_fb_helper_output_poll_changed,
 	.atomic_check = drm_atomic_helper_check,
 	.atomic_commit = drm_atomic_helper_commit,
 };
@@ -41,7 +40,6 @@ DEFINE_DRM_GEM_CMA_FOPS(zx_drm_fops);
 static struct drm_driver zx_drm_driver = {
 	.driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME |
 			   DRIVER_ATOMIC,
-	.lastclose = drm_fb_helper_lastclose,
 	.gem_free_object_unlocked = drm_gem_cma_free_object,
 	.gem_vm_ops = &drm_gem_cma_vm_ops,
 	.dumb_create = drm_gem_cma_dumb_create,
@@ -101,20 +99,14 @@ static int zx_drm_bind(struct device *dev)
 	drm_mode_config_reset(drm);
 	drm_kms_helper_poll_init(drm);
 
-	ret = drm_fb_cma_fbdev_init(drm, 32, 0);
-	if (ret) {
-		DRM_DEV_ERROR(dev, "failed to init cma fbdev: %d\n", ret);
-		goto out_poll_fini;
-	}
-
 	ret = drm_dev_register(drm, 0);
 	if (ret)
-		goto out_fbdev_fini;
+		goto out_poll_fini;
+
+	drm_fbdev_generic_setup(drm, 32);
 
 	return 0;
 
-out_fbdev_fini:
-	drm_fb_cma_fbdev_fini(drm);
 out_poll_fini:
 	drm_kms_helper_poll_fini(drm);
 	drm_mode_config_cleanup(drm);
@@ -131,7 +123,6 @@ static void zx_drm_unbind(struct device *dev)
 	struct drm_device *drm = dev_get_drvdata(dev);
 
 	drm_dev_unregister(drm);
-	drm_fb_cma_fbdev_fini(drm);
 	drm_kms_helper_poll_fini(drm);
 	drm_mode_config_cleanup(drm);
 	component_unbind_all(dev, drm);
-- 
2.15.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 19/20] drm/zte: Use drm_fbdev_generic_setup()

[Shawn Guo]

On Sat, Sep 08, 2018 at 03:46:47PM +0200, Noralf Trønnes wrote:
> The CMA helper is already using the drm_fb_helper_generic_probe part of
> the generic fbdev emulation. This patch makes full use of the generic
> fbdev emulation by using its drm_client callbacks. This means that
> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
> now handled by the emulation code. Additionally fbdev unregister happens
> automatically on drm_dev_unregister().
> 
> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
> driver. This is done to highlight the fact that fbdev emulation is an
> internal client that makes use of the driver, it is not part of the
> driver as such. If fbdev setup fails, an error is printed, but the driver
> succeeds probing.
> 
> Cc: Shawn Guo <shawnguo@kernel.org>
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

Acked-by: Shawn Guo <shawnguo@kernel.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 19/20] drm/zte: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

Den 10.09.2018 03.23, skrev Shawn Guo:
> On Sat, Sep 08, 2018 at 03:46:47PM +0200, Noralf Trønnes wrote:
>> The CMA helper is already using the drm_fb_helper_generic_probe part of
>> the generic fbdev emulation. This patch makes full use of the generic
>> fbdev emulation by using its drm_client callbacks. This means that
>> drm_mode_config_funcs->output_poll_changed and drm_driver->lastclose are
>> now handled by the emulation code. Additionally fbdev unregister happens
>> automatically on drm_dev_unregister().
>>
>> The drm_fbdev_generic_setup() call is put after drm_dev_register() in the
>> driver. This is done to highlight the fact that fbdev emulation is an
>> internal client that makes use of the driver, it is not part of the
>> driver as such. If fbdev setup fails, an error is printed, but the driver
>> succeeds probing.
>>
>> Cc: Shawn Guo <shawnguo@kernel.org>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> Acked-by: Shawn Guo <shawnguo@kernel.org>

Applied to drm-misc-next, thanks.

Noralf

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 20/20] drm/cma-helper: Remove unused fbdev code

[Noralf Trønnes]

CMA helper drivers have been converted to drm_fbdev_generic_setup()
so the fbdev code can be removed.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/Kconfig             |   4 -
 drivers/gpu/drm/drm_fb_cma_helper.c | 151 ------------------------------------
 include/drm/drm_fb_cma_helper.h     |  24 ------
 3 files changed, 179 deletions(-)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index cb88528e7b10..8871b3fa8832 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -150,10 +150,6 @@ config DRM_KMS_CMA_HELPER
 	bool
 	depends on DRM
 	select DRM_GEM_CMA_HELPER
-	select DRM_KMS_FB_HELPER
-	select FB_SYS_FILLRECT
-	select FB_SYS_COPYAREA
-	select FB_SYS_IMAGEBLIT
 	help
 	  Choose this if you need the KMS CMA helper functions
 
diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c
index 47e0e2f6642d..0ddf9c65e5ab 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -18,8 +18,6 @@
  */
 
 #include <drm/drmP.h>
-#include <drm/drm_client.h>
-#include <drm/drm_fb_helper.h>
 #include <drm/drm_framebuffer.h>
 #include <drm/drm_gem_cma_helper.h>
 #include <drm/drm_gem_framebuffer_helper.h>
@@ -27,10 +25,6 @@
 #include <drm/drm_print.h>
 #include <linux/module.h>
 
-struct drm_fbdev_cma {
-	struct drm_fb_helper	fb_helper;
-};
-
 /**
  * DOC: framebuffer cma helper functions
  *
@@ -39,16 +33,8 @@ struct drm_fbdev_cma {
  *
  * drm_gem_fb_create() is used in the &drm_mode_config_funcs.fb_create
  * callback function to create a cma backed framebuffer.
- *
- * An fbdev framebuffer backed by cma is also available by calling
- * drm_fb_cma_fbdev_init(). drm_fb_cma_fbdev_fini() tears it down.
  */
 
-static inline struct drm_fbdev_cma *to_fbdev_cma(struct drm_fb_helper *helper)
-{
-	return container_of(helper, struct drm_fbdev_cma, fb_helper);
-}
-
 /**
  * drm_fb_cma_get_gem_obj() - Get CMA GEM object for framebuffer
  * @fb: The framebuffer
@@ -105,140 +91,3 @@ dma_addr_t drm_fb_cma_get_gem_addr(struct drm_framebuffer *fb,
 	return paddr;
 }
 EXPORT_SYMBOL_GPL(drm_fb_cma_get_gem_addr);
-
-/**
- * drm_fb_cma_fbdev_init() - Allocate and initialize fbdev emulation
- * @dev: DRM device
- * @preferred_bpp: Preferred bits per pixel for the device.
- *                 @dev->mode_config.preferred_depth is used if this is zero.
- * @max_conn_count: Maximum number of connectors.
- *                  @dev->mode_config.num_connector is used if this is zero.
- *
- * Returns:
- * Zero on success or negative error code on failure.
- */
-int drm_fb_cma_fbdev_init(struct drm_device *dev, unsigned int preferred_bpp,
-			  unsigned int max_conn_count)
-{
-	struct drm_fbdev_cma *fbdev_cma;
-
-	/* dev->fb_helper will indirectly point to fbdev_cma after this call */
-	fbdev_cma = drm_fbdev_cma_init(dev, preferred_bpp, max_conn_count);
-	if (IS_ERR(fbdev_cma))
-		return PTR_ERR(fbdev_cma);
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(drm_fb_cma_fbdev_init);
-
-/**
- * drm_fb_cma_fbdev_fini() - Teardown fbdev emulation
- * @dev: DRM device
- */
-void drm_fb_cma_fbdev_fini(struct drm_device *dev)
-{
-	if (dev->fb_helper)
-		drm_fbdev_cma_fini(to_fbdev_cma(dev->fb_helper));
-}
-EXPORT_SYMBOL_GPL(drm_fb_cma_fbdev_fini);
-
-static const struct drm_fb_helper_funcs drm_fb_cma_helper_funcs = {
-	.fb_probe = drm_fb_helper_generic_probe,
-};
-
-/**
- * drm_fbdev_cma_init() - Allocate and initializes a drm_fbdev_cma struct
- * @dev: DRM device
- * @preferred_bpp: Preferred bits per pixel for the device
- * @max_conn_count: Maximum number of connectors
- *
- * Returns a newly allocated drm_fbdev_cma struct or a ERR_PTR.
- */
-struct drm_fbdev_cma *drm_fbdev_cma_init(struct drm_device *dev,
-	unsigned int preferred_bpp, unsigned int max_conn_count)
-{
-	struct drm_fbdev_cma *fbdev_cma;
-	struct drm_fb_helper *fb_helper;
-	int ret;
-
-	fbdev_cma = kzalloc(sizeof(*fbdev_cma), GFP_KERNEL);
-	if (!fbdev_cma)
-		return ERR_PTR(-ENOMEM);
-
-	fb_helper = &fbdev_cma->fb_helper;
-
-	ret = drm_client_new(dev, &fb_helper->client, "fbdev", NULL);
-	if (ret)
-		goto err_free;
-
-	ret = drm_fb_helper_fbdev_setup(dev, fb_helper, &drm_fb_cma_helper_funcs,
-					preferred_bpp, max_conn_count);
-	if (ret)
-		goto err_client_put;
-
-	return fbdev_cma;
-
-err_client_put:
-	drm_client_release(&fb_helper->client);
-err_free:
-	kfree(fbdev_cma);
-
-	return ERR_PTR(ret);
-}
-EXPORT_SYMBOL_GPL(drm_fbdev_cma_init);
-
-/**
- * drm_fbdev_cma_fini() - Free drm_fbdev_cma struct
- * @fbdev_cma: The drm_fbdev_cma struct
- */
-void drm_fbdev_cma_fini(struct drm_fbdev_cma *fbdev_cma)
-{
-	drm_fb_helper_unregister_fbi(&fbdev_cma->fb_helper);
-	/* All resources have now been freed by drm_fbdev_fb_destroy() */
-}
-EXPORT_SYMBOL_GPL(drm_fbdev_cma_fini);
-
-/**
- * drm_fbdev_cma_restore_mode() - Restores initial framebuffer mode
- * @fbdev_cma: The drm_fbdev_cma struct, may be NULL
- *
- * This function is usually called from the &drm_driver.lastclose callback.
- */
-void drm_fbdev_cma_restore_mode(struct drm_fbdev_cma *fbdev_cma)
-{
-	if (fbdev_cma)
-		drm_fb_helper_restore_fbdev_mode_unlocked(&fbdev_cma->fb_helper);
-}
-EXPORT_SYMBOL_GPL(drm_fbdev_cma_restore_mode);
-
-/**
- * drm_fbdev_cma_hotplug_event() - Poll for hotpulug events
- * @fbdev_cma: The drm_fbdev_cma struct, may be NULL
- *
- * This function is usually called from the &drm_mode_config.output_poll_changed
- * callback.
- */
-void drm_fbdev_cma_hotplug_event(struct drm_fbdev_cma *fbdev_cma)
-{
-	if (fbdev_cma)
-		drm_fb_helper_hotplug_event(&fbdev_cma->fb_helper);
-}
-EXPORT_SYMBOL_GPL(drm_fbdev_cma_hotplug_event);
-
-/**
- * drm_fbdev_cma_set_suspend_unlocked - wrapper around
- *                                      drm_fb_helper_set_suspend_unlocked
- * @fbdev_cma: The drm_fbdev_cma struct, may be NULL
- * @state: desired state, zero to resume, non-zero to suspend
- *
- * Calls drm_fb_helper_set_suspend, which is a wrapper around
- * fb_set_suspend implemented by fbdev core.
- */
-void drm_fbdev_cma_set_suspend_unlocked(struct drm_fbdev_cma *fbdev_cma,
-					bool state)
-{
-	if (fbdev_cma)
-		drm_fb_helper_set_suspend_unlocked(&fbdev_cma->fb_helper,
-						   state);
-}
-EXPORT_SYMBOL(drm_fbdev_cma_set_suspend_unlocked);
diff --git a/include/drm/drm_fb_cma_helper.h b/include/drm/drm_fb_cma_helper.h
index 4a65f0d155b0..4becb09975a4 100644
--- a/include/drm/drm_fb_cma_helper.h
+++ b/include/drm/drm_fb_cma_helper.h
@@ -2,33 +2,9 @@
 #ifndef __DRM_FB_CMA_HELPER_H__
 #define __DRM_FB_CMA_HELPER_H__
 
-struct drm_fbdev_cma;
-struct drm_gem_cma_object;
-
-struct drm_fb_helper_surface_size;
-struct drm_framebuffer_funcs;
-struct drm_fb_helper_funcs;
 struct drm_framebuffer;
-struct drm_fb_helper;
-struct drm_device;
-struct drm_file;
-struct drm_mode_fb_cmd2;
-struct drm_plane;
 struct drm_plane_state;
 
-int drm_fb_cma_fbdev_init(struct drm_device *dev, unsigned int preferred_bpp,
-			  unsigned int max_conn_count);
-void drm_fb_cma_fbdev_fini(struct drm_device *dev);
-
-struct drm_fbdev_cma *drm_fbdev_cma_init(struct drm_device *dev,
-	unsigned int preferred_bpp, unsigned int max_conn_count);
-void drm_fbdev_cma_fini(struct drm_fbdev_cma *fbdev_cma);
-
-void drm_fbdev_cma_restore_mode(struct drm_fbdev_cma *fbdev_cma);
-void drm_fbdev_cma_hotplug_event(struct drm_fbdev_cma *fbdev_cma);
-void drm_fbdev_cma_set_suspend_unlocked(struct drm_fbdev_cma *fbdev_cma,
-					bool state);
-
 struct drm_gem_cma_object *drm_fb_cma_get_gem_obj(struct drm_framebuffer *fb,
 	unsigned int plane);
 
-- 
2.15.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/20] drm/cma-helper drivers: Use drm_fbdev_generic_setup()

[Sam Ravnborg]

Hi Noralf.

On Sat, Sep 08, 2018 at 03:46:28PM +0200, Noralf Trønnes wrote:
> This patchset moves the drivers using the CMA helper fully over to the
> generic fbdev emulation. The unused fbdev code is removed from the CMA
> helper.

The full series looks good.
I have browsed all the code changes, and with one minor
note where one patch could have been split up all the rest looked good.
Feel free to add my:
Acked-by: Sam Ravnborg <sam@ravnborg.org>
to the full series.


>  32 files changed, 56 insertions(+), 492 deletions(-)

This kind of diffstat is always a pleasure to see.
Especially as in this case where functionality is not
removed, just made smarter.

I know this set does not include the infrastructure parts,
so in reality more lines added. But anyway.

	Sam
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/20] drm/cma-helper drivers: Use drm_fbdev_generic_setup()

[Noralf Trønnes]

Cc: dri-devel and Sam (sorry about the slip)


Den 10.09.2018 14.53, skrev Noralf Trønnes:
>
> Den 08.09.2018 15.46, skrev Noralf Trønnes:
>> This patchset moves the drivers using the CMA helper fully over to the
>> generic fbdev emulation. The unused fbdev code is removed from the CMA
>> helper.
>
> Maxime asked for context so I'm replying to all:
>
> This is the base for this patchset which added 
> drm_fb_helper_generic_probe,
> it was applied in July (now in 4.19):
>
> drm: Add generic fbdev emulation
> https://patchwork.freedesktop.org/series/45848/
>
> Noralf.
>
>> Noralf.
>>
>> Noralf Trønnes (20):
>>    drm/fb-helper: Improve error reporting in setup
>>    drm/arc: Use drm_fbdev_generic_setup()
>>    drm/fsl-dcu: Use drm_fbdev_generic_setup()
>>    drm/hisilicon/kirin: Use drm_fbdev_generic_setup()
>>    drm/meson: Use drm_fbdev_generic_setup()
>>    drm/mxsfb: Use drm_fbdev_generic_setup()
>>    drm/rcar-du: Use drm_fbdev_generic_setup()
>>    drm/arm/hdlcd: Use drm_fbdev_generic_setup()
>>    drm/arm/mali: Use drm_fbdev_generic_setup()
>>    drm/atmel-hlcdc: Use drm_fbdev_generic_setup()
>>    drm/imx: Use drm_fbdev_generic_setup()
>>    drm/pl111: Use drm_fbdev_generic_setup()
>>    drm/sti: Use drm_fbdev_generic_setup()
>>    drm/stm: Use drm_fbdev_generic_setup()
>>    drm/sun4i: Use drm_fbdev_generic_setup()
>>    drm/tilcdc: Use drm_fbdev_generic_setup()
>>    drm/tve200: Use drm_fbdev_generic_setup()
>>    drm/vc4: Use drm_fbdev_generic_setup()
>>    drm/zte: Use drm_fbdev_generic_setup()
>>    drm/cma-helper: Remove unused fbdev code
>>
>>   drivers/gpu/drm/Kconfig                         |   4 -
>>   drivers/gpu/drm/arc/arcpgu.h                    |   4 -
>>   drivers/gpu/drm/arc/arcpgu_drv.c                |  33 +-----
>>   drivers/gpu/drm/arm/hdlcd_drv.c                 |  11 +-
>>   drivers/gpu/drm/arm/malidp_drv.c                |  11 +-
>>   drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c    |   7 +-
>>   drivers/gpu/drm/drm_fb_cma_helper.c             | 151 
>> ------------------------
>>   drivers/gpu/drm/drm_fb_helper.c                 |   8 +-
>>   drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c       |  25 +---
>>   drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h       |   1 -
>>   drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c |  38 +-----
>>   drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h |   4 -
>>   drivers/gpu/drm/imx/imx-drm-core.c              |  19 +--
>>   drivers/gpu/drm/meson/meson_drv.c               |  19 +--
>>   drivers/gpu/drm/meson/meson_drv.h               |   1 -
>>   drivers/gpu/drm/mxsfb/mxsfb_drv.c               |  26 +---
>>   drivers/gpu/drm/mxsfb/mxsfb_drv.h               |   1 -
>>   drivers/gpu/drm/pl111/pl111_drv.c               |   6 +-
>>   drivers/gpu/drm/rcar-du/rcar_du_drv.c           |  34 +-----
>>   drivers/gpu/drm/rcar-du/rcar_du_drv.h           |   3 -
>>   drivers/gpu/drm/rcar-du/rcar_du_kms.c           |  21 ----
>>   drivers/gpu/drm/sti/sti_drv.c                   |   8 +-
>>   drivers/gpu/drm/stm/drv.c                       |  11 +-
>>   drivers/gpu/drm/sun4i/sun4i_drv.c               |  12 +-
>>   drivers/gpu/drm/sun4i/sun4i_framebuffer.c       |  12 +-
>>   drivers/gpu/drm/sun4i/sun4i_framebuffer.h       |   3 +-
>>   drivers/gpu/drm/tilcdc/tilcdc_drv.c             |  11 +-
>>   drivers/gpu/drm/tve200/tve200_drv.c             |  14 +--
>>   drivers/gpu/drm/vc4/vc4_drv.c                   |   5 +-
>>   drivers/gpu/drm/vc4/vc4_kms.c                   |   6 -
>>   drivers/gpu/drm/zte/zx_drm_drv.c                |  15 +--
>>   include/drm/drm_fb_cma_helper.h                 |  24 ----
>>   32 files changed, 56 insertions(+), 492 deletions(-)
>>
>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel