| Message ID | 20171019230213.63521-3-noralf@tronnes.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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() Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > 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; > }; > > #endif > -- > 2.14.2 >
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
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
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@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. 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@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
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@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. > 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@lists.freedesktop.org >>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Fri, Oct 20, 2017 at 07:11:18PM +0200, Noralf Trønnes wrote: > > 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@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. > > 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? I retract my bikeshed, looks like you&Ville are much more informed :-) -Daniel > > 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@lists.freedesktop.org > > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel >
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; }; #endif
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(-)