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 ;)
>
> 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
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
