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

Noralf Trønnes
2017-10-20T15:44:15Z

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@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.

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.

fb_helper is the common variable name used in drm_fb_helper (which
really should have been called drm_fbdev_helper imo).

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@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel