[PATCH 03/22] drm/arc: Use drm_fb_cma_fbdev_init/fini()

Daniel Vetter daniel at ffwll.ch
Mon Nov 6 09:08:03 UTC 2017


On Sat, Nov 04, 2017 at 02:03:57PM +0100, Noralf Trønnes wrote:
> Use drm_fb_cma_fbdev_init() and drm_fb_cma_fbdev_fini() which relies on
> the fact that drm_device holds a pointer to the drm_fb_helper structure.
> This means that the driver doesn't have to keep track of that.
> Also use the drm_fb_helper functions directly.
> Remove unused function prototype arcpgu_fbdev_cma_init().
> 
> Cc: Alexey Brodkin <abrodkin at synopsys.com>
> Signed-off-by: Noralf Trønnes <noralf at tronnes.org>
> ---
>  drivers/gpu/drm/arc/arcpgu.h     |  4 ----
>  drivers/gpu/drm/arc/arcpgu_drv.c | 36 +++++++-----------------------------
>  2 files changed, 7 insertions(+), 33 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 074fd4ea7ece..f54481ee834c 100644
> --- a/drivers/gpu/drm/arc/arcpgu_drv.c
> +++ b/drivers/gpu/drm/arc/arcpgu_drv.c
> @@ -16,6 +16,7 @@
>  
>  #include <linux/clk.h>
>  #include <drm/drm_crtc_helper.h>
> +#include <drm/drm_fb_helper.h>
>  #include <drm/drm_fb_cma_helper.h>
>  #include <drm/drm_gem_cma_helper.h>
>  #include <drm/drm_gem_framebuffer_helper.h>
> @@ -25,16 +26,9 @@
>  #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,
> +	.output_poll_changed = drm_fb_helper_output_poll_changed,
>  	.atomic_check = drm_atomic_helper_check,
>  	.atomic_commit = drm_atomic_helper_commit,
>  };
> @@ -51,13 +45,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,13 +100,9 @@ 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;
> -	}
> +	ret = drm_fb_cma_fbdev_init(drm, 16, 0);

s/0/NULL/

Probably applies to other patches too.

Also, why did you merge this new change into the old patch with the
output_poll/lastclose change? It took me a while to figure out what you're
doing, because the patches looked familiar, but the cover letter didn't
mention how stuff evolved, nor the patches here.

Also, you've lost the bunch of acks/r-bs you've gathered already. If
there's no technical reason to merge the 2 patches I'm kinda leaning
towards merging them separately, at least the ones that have acks already.
Just to avoid the pitfall of the continually evolving patch series that
never lands.
-Daniel

> +	if (ret)
> +		return ret;
>  
>  	platform_set_drvdata(pdev, drm);
>  	return 0;
> @@ -127,12 +110,7 @@ static int arcpgu_load(struct drm_device *drm)
>  
>  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_fb_cma_fbdev_fini(drm);
>  	drm_kms_helper_poll_fini(drm);
>  	drm_mode_config_cleanup(drm);
>  
> @@ -168,7 +146,7 @@ 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,
> +	.lastclose = drm_fb_helper_lastclose,
>  	.name = "arcpgu",
>  	.desc = "ARC PGU Controller",
>  	.date = "20160219",
> -- 
> 2.14.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


More information about the dri-devel mailing list