Re: [PATCH 02/15] drm: Add drm_device->drm_fb_helper_private pointer

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: "Noralf Trønnes" <noralf@tronnes.org>
Cc: daniel.vetter@ffwll.ch, intel-gfx@lists.freedesktop.org,
	joonas.lahtinen@linux.intel.com, sw0312.kim@samsung.com,
	linux@armlinux.org.uk, dri-devel@lists.freedesktop.org,
	hdegoede@redhat.com, kyungmin.park@samsung.com,
	tomi.valkeinen@ti.com, thierry.reding@gmail.com,
	bskeggs@redhat.com, rodrigo.vivi@intel.com,
	alexander.deucher@amd.com, christian.koenig@amd.com
Subject: Re: [PATCH 02/15] drm: Add drm_device->drm_fb_helper_private pointer
Date: Fri, 20 Oct 2017 16:52:12 +0300	[thread overview]
Message-ID: <20171020135212.GH10981@intel.com> (raw)
In-Reply-To: <20171019230213.63521-3-noralf@tronnes.org>

On Fri, Oct 20, 2017 at 01:02:00AM +0200, Noralf Trønnes wrote:
> drm_fb_helper is *the* way of doing fbdev emulation so add a pointer to
> struct drm_device. This makes it possible to add callback helpers for
> .last_close and .output_poll_changed further reducing fbdev emulation
> footprint in drivers. The pointer is set by drm_fb_helper_init() and
> cleared by drm_fb_helper_fini().
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
> 
> This patch was initially part of the patchset: drm/tinydrm: Use vmalloc BO
> Changes since previous version:
> - Change member name: fbdev -> drm_fb_helper_private
> - Expand docs
> - Set and clear pointer in drm_fb_helper_init/fini()
> 
>  drivers/gpu/drm/drm_fb_helper.c | 13 +++++++++++--
>  include/drm/drm_device.h        |  9 +++++++++
>  2 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 954cdd48de92..c07b7af8de4c 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -799,8 +799,10 @@ int drm_fb_helper_init(struct drm_device *dev,
>  	struct drm_mode_config *config = &dev->mode_config;
>  	int i;
>  
> -	if (!drm_fbdev_emulation)
> +	if (!drm_fbdev_emulation) {
> +		dev->drm_fb_helper_private = fb_helper;
>  		return 0;
> +	}
>  
>  	if (!max_conn_count)
>  		return -EINVAL;
> @@ -835,6 +837,8 @@ int drm_fb_helper_init(struct drm_device *dev,
>  		i++;
>  	}
>  
> +	dev->drm_fb_helper_private = fb_helper;
> +
>  	return 0;
>  out_free:
>  	drm_fb_helper_crtc_free(fb_helper);
> @@ -913,7 +917,12 @@ void drm_fb_helper_fini(struct drm_fb_helper *fb_helper)
>  {
>  	struct fb_info *info;
>  
> -	if (!drm_fbdev_emulation || !fb_helper)
> +	if (!fb_helper)
> +		return;
> +
> +	fb_helper->dev->drm_fb_helper_private = NULL;
> +
> +	if (!drm_fbdev_emulation)
>  		return;
>  
>  	cancel_work_sync(&fb_helper->resume_work);
> diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
> index e21af87a2f3c..6b26262658ae 100644
> --- a/include/drm/drm_device.h
> +++ b/include/drm/drm_device.h
> @@ -17,6 +17,7 @@ struct drm_vblank_crtc;
>  struct drm_sg_mem;
>  struct drm_local_map;
>  struct drm_vma_offset_manager;
> +struct drm_fb_helper;
>  
>  struct inode;
>  
> @@ -185,6 +186,14 @@ struct drm_device {
>  	struct drm_vma_offset_manager *vma_offset_manager;
>  	/*@} */
>  	int switch_power_state;
> +
> +	/**
> +	 * @drm_fb_helper_private:
> +	 *
> +	 * Pointer to the fbdev emulation structure.
> +	 * Set by drm_fb_helper_init() and cleared by drm_fb_helper_fini().
> +	 */
> +	struct drm_fb_helper *drm_fb_helper_private;

Just 'fb_helper' maybe? Not sure what the _private here is supposed to
mean, and drm_ seems very much redundant.

>  };
>  
>  #endif
> -- 
> 2.14.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel