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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Den 20.10.2017 18.00, skrev Ville Syrjälä:
On Fri, Oct 20, 2017 at 05:44:15PM +0200, Noralf Trønnes wrote:
Den 20.10.2017 15.52, skrev Ville Syrjälä:
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@xxxxxxxxxxx>
---

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.
I first called it fbdev, Daniel suggested fbdev_helper_private, then I
took it all the way and called it drm_fb_helper_private. I believe the
_private part is an indication that this is not part of the core, but a
helper, like drm_plane->helper_private.
IMO those we should rename to helper_funcs since that's what they always
are. IIRC they used to be void* and then the _private made more sense to
me since you couldn't directly know what they were pointing at.

To me _private means that it's totally up to driver to specify what to
put there (eg. like we have private_flags in the mode structure).

And I believe the "helper" part in "fb_helper" should be enough of a hint
to anyone that this has something to do with a helper ;)

Daniel, do you have any objections to calling it: fb_helper?

Noralf.

fb_helper is the common variable name used in drm_fb_helper (which
really should have been called drm_fbdev_helper imo).
Yeah. On a first glance one might think drm_fb_helper has something
to do with drm_framebuffers.

These are the names that drivers use:

      struct amdgpu_fbdev *rfbdev;
      struct drm_fb_helper    *fbdev;
      struct ast_fbdev *fbdev;
      struct drm_fb_helper fb.helper;
      struct cirrus_fbdev        *gfbdev;
      struct drm_fb_helper    fb_helper;
      struct drm_fb_helper *fb_helper;
      void *fbdev;
      struct hibmc_fbdev *fbdev;
      struct mga_fbdev *mfbdev;
      struct drm_fb_helper *fbdev;
      struct nouveau_fbdev *fbcon;
      struct drm_fb_helper *fbdev;
      struct qxl_fbdev *qfbdev;
      struct radeon_fbdev *rfbdev;
      struct drm_fb_helper fbdev_helper;
      struct tegra_fbdev *fbdev;
      struct udl_fbdev *fbdev;
      struct virtio_gpu_fbdev *vgfbdev;
      struct vbox_fbdev *fbdev;

Noralf.

   };
#endif
--
2.14.2

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

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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux