Any updates on this or too much distractions? I really think doing
this would be awesome for the drm subsystem, instead of reinventing
this wheel for each driver.
-Daniel
On Wed, Mar 25, 2015 at 10:21 AM, Daniel Vetter <[email protected]> wrote:
> On Wed, Mar 25, 2015 at 01:47:54PM +0530, Archit Taneja wrote:
>> Hi,
>>
>> On 03/13/2015 02:36 PM, Daniel Vetter wrote:
>> >On Fri, Mar 13, 2015 at 11:55:07AM +0530, Archit Taneja wrote:
>> >>
>> >>
>> >>On 03/11/2015 08:47 PM, Daniel Vetter wrote:
>> >>>On Wed, Mar 11, 2015 at 01:51:02PM +0530, Archit Taneja wrote:
>> >>>>
>> >>>>
>> >>>>On 03/10/2015 05:47 PM, Daniel Vetter wrote:
>> >>>>>On Tue, Mar 10, 2015 at 03:52:41PM +0530, Archit Taneja wrote:
>> >>>>>>On 03/10/2015 03:35 PM, Daniel Vetter wrote:
>> >>>>>>>On Tue, Mar 10, 2015 at 03:22:49PM +0530, Archit Taneja wrote:
>> >>>>>>>>On 03/10/2015 03:17 PM, Daniel Vetter wrote:
>> >>>>>>>>>On Tue, Mar 10, 2015 at 03:11:28PM +0530, Archit Taneja wrote:
>> >>>>>>>>>>diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>> >>>>>>>>>>index 151a050..38f83a0 100644
>> >>>>>>>>>>--- a/drivers/gpu/drm/Kconfig
>> >>>>>>>>>>+++ b/drivers/gpu/drm/Kconfig
>> >>>>>>>>>>@@ -40,6 +40,24 @@ config DRM_KMS_FB_HELPER
>> >>>>>>>>>> help
>> >>>>>>>>>> FBDEV helpers for KMS drivers.
>> >>>>>>>>>>
>> >>>>>>>>>>+config DRM_FBDEV_EMULATION
>> >>>>>>>>>>+ bool "Enable legacy fbdev support for your modesetting driver"
>> >>>>>>>>>>+ depends on DRM
>> >>>>>>>>>>+ select DRM_KMS_HELPER
>> >>>>>>>>>>+ select DRM_KMS_FB_HELPER
>> >>>>>>>>>>+ select FB_SYS_FILLRECT
>> >>>>>>>>>>+ select FB_SYS_COPYAREA
>> >>>>>>>>>>+ select FB_SYS_IMAGEBLIT
>> >>>>>>>>>>+ select FB_SYS_FOPS
>> >>>>>>>>>>+ select FB_CFB_FILLRECT
>> >>>>>>>>>>+ select FB_CFB_COPYAREA
>> >>>>>>>>>>+ select FB_CFB_IMAGEBLIT
>> >>>>>>>>>>+ default y
>> >>>>>>>>>>+ help
>> >>>>>>>>>>+ Choose this option if you have a need for the legacy fbdev
>> >>>>>>>>>>+ support. Note that this support also provide the linux console
>> >>>>>>>>>>+ support on top of your modesetting driver.
>> >>>>>>>>>
>> >>>>>>>>>Maybe clarify that for linux console support you also need
>> >>>>>>>>>CONFIG_FRAMEBUFFER_CONSOLE? fbdev alone isn't enough.
>> >>>>>>>>
>> >>>>>>>>DRM_KMS_FB_HELPER selects that for us, right?
>> >>>>>>>
>> >>>>>>>Hm right I've missed that. Reminds me that you need one more patch at the
>> >>>>>>>end to remove all the various select DRM_KMS_FB_HELPER from all drm
>> >>>>>>>drivers. Otherwise this knob here won't work by default if you e.g. select
>> >>>>>>>radeon. In general we can't mix explicit options with menu entries with a
>> >>>>>>>select.
>> >>>>>>
>> >>>>>>I was trying that out. Removing DRM_KMS_FB_HELPER and having
>> >>>>>>DRM_FBDEV_EMULATION disabled breaks drivers which use FB stuff internally in
>> >>>>>>their respective xyz_fbdev.c files.
>> >>>>>
>> >>>>>But with the stubbed out functions that should work, right? Why doesn't
>> >>>>>it?
>> >>>>
>> >>>>There are still calls to functions from fb core like fb_set_suspend and
>> >>>>register_framebuffer which aren't covered by the drm fb helper functions.
>> >>>
>> >>>Hm, sounds like we need another patch to stub out fb_set_suspend when
>> >>>fbdev isn't enabled. Is there anything else?
>> >>
>> >>There are a handful of fb core functions which are called by drm drivers:
>> >>
>> >>fb_alloc_cmap/fb_dealloc_cmap
>> >>
>> >>fb_sys_read/fb_sys_write
>> >>
>> >>register_framebuffer/unregister_framebuffer/unlink_framebuffer/
>> >>remove_conflicting_framebuffers
>> >>
>> >>fb_set_suspend
>> >>
>> >>fb_deferred_io_init/fb_deferred_io_cleanup
>> >>
>> >>framebuffer_alloc/framebuffer_release
>> >
>> >Hm yeah that's somewhat annoying indeed. What about the following:
>> >1. We move all the #include <linux/fb.h> from drivers into drm_fb_helper.h
>> >
>> >2. Then we add stubs for these functions in drm_fb_helper.h, like this
>> >
>> >#if defined(CONFIG_FB)
>> >#include <linux/fb.h>
>> >#else
>> >
>> >/* static inline stubs for all the fb stuff used by kms drivers */
>> >#endif
>> >
>> >Imo this makes sense since kms drivers really have a bit a special
>> >situation with fbdev. They're not full-blown fbdev drivers and can be
>> >useful fully without fbdev.
>> >
>>
>> I was trying this out. Removing 'linux/fb.h' and replacing stub fb funcs
>> won't really work because struct declarations(like fb_info) also get
>> removed.
>>
>> I considered placing '#if IS_ENABLED(CONFIG_FB)' within linux/fb.h itself,
>> but that seemed a bit too intrusive.
>>
>> This is what I'm currently doing:
>>
>> - Some funcs, like framebufer_alloc/release, alloc_cmap/dealloc_cmap would
>> actually benefit if we have drm fb helpers for them. They are used in
>> exactly the same manner by all the drivers.
>>
>> - For the rest of the functions that are sparsely used, I was considering
>> making very simple drm_fb_* wrapper functions. Something like:
>>
>> void drm_fb_helper_deferred_io_init(struct drm_fb_helper *helper)
>> {
>> if (helper->fbdev)
>> fb_deferred_io_init(helper->fbdev);
>> }
>>
>> We could have all fb calls called within drm_fb_helper.c, creating
>> drm_fb_helper_* stub functions would then be an easier task. What do you
>> think?
>
> That's actually an option I considered, but I hoped we could do it with
> less work. If this indeed works and you can create the patch that would be
> awesome.
>
> Thanks, Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
Hi,
On 06/30/2015 12:40 PM, Daniel Vetter wrote:
> Any updates on this or too much distractions? I really think doing
> this would be awesome for the drm subsystem, instead of reinventing
> this wheel for each driver.
I'd started on this again. I've got more free time now than before, so I
should have something in a week or so. I agree it will help a lot (there
are already two new drivers since we started discussing this!)
Archit
> -Daniel
>
> On Wed, Mar 25, 2015 at 10:21 AM, Daniel Vetter <[email protected]> wrote:
>> On Wed, Mar 25, 2015 at 01:47:54PM +0530, Archit Taneja wrote:
>>> Hi,
>>>
>>> On 03/13/2015 02:36 PM, Daniel Vetter wrote:
>>>> On Fri, Mar 13, 2015 at 11:55:07AM +0530, Archit Taneja wrote:
>>>>>
>>>>>
>>>>> On 03/11/2015 08:47 PM, Daniel Vetter wrote:
>>>>>> On Wed, Mar 11, 2015 at 01:51:02PM +0530, Archit Taneja wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 03/10/2015 05:47 PM, Daniel Vetter wrote:
>>>>>>>> On Tue, Mar 10, 2015 at 03:52:41PM +0530, Archit Taneja wrote:
>>>>>>>>> On 03/10/2015 03:35 PM, Daniel Vetter wrote:
>>>>>>>>>> On Tue, Mar 10, 2015 at 03:22:49PM +0530, Archit Taneja wrote:
>>>>>>>>>>> On 03/10/2015 03:17 PM, Daniel Vetter wrote:
>>>>>>>>>>>> On Tue, Mar 10, 2015 at 03:11:28PM +0530, Archit Taneja wrote:
>>>>>>>>>>>>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>>>>>>>>>>>>> index 151a050..38f83a0 100644
>>>>>>>>>>>>> --- a/drivers/gpu/drm/Kconfig
>>>>>>>>>>>>> +++ b/drivers/gpu/drm/Kconfig
>>>>>>>>>>>>> @@ -40,6 +40,24 @@ config DRM_KMS_FB_HELPER
>>>>>>>>>>>>> help
>>>>>>>>>>>>> FBDEV helpers for KMS drivers.
>>>>>>>>>>>>>
>>>>>>>>>>>>> +config DRM_FBDEV_EMULATION
>>>>>>>>>>>>> + bool "Enable legacy fbdev support for your modesetting driver"
>>>>>>>>>>>>> + depends on DRM
>>>>>>>>>>>>> + select DRM_KMS_HELPER
>>>>>>>>>>>>> + select DRM_KMS_FB_HELPER
>>>>>>>>>>>>> + select FB_SYS_FILLRECT
>>>>>>>>>>>>> + select FB_SYS_COPYAREA
>>>>>>>>>>>>> + select FB_SYS_IMAGEBLIT
>>>>>>>>>>>>> + select FB_SYS_FOPS
>>>>>>>>>>>>> + select FB_CFB_FILLRECT
>>>>>>>>>>>>> + select FB_CFB_COPYAREA
>>>>>>>>>>>>> + select FB_CFB_IMAGEBLIT
>>>>>>>>>>>>> + default y
>>>>>>>>>>>>> + help
>>>>>>>>>>>>> + Choose this option if you have a need for the legacy fbdev
>>>>>>>>>>>>> + support. Note that this support also provide the linux console
>>>>>>>>>>>>> + support on top of your modesetting driver.
>>>>>>>>>>>>
>>>>>>>>>>>> Maybe clarify that for linux console support you also need
>>>>>>>>>>>> CONFIG_FRAMEBUFFER_CONSOLE? fbdev alone isn't enough.
>>>>>>>>>>>
>>>>>>>>>>> DRM_KMS_FB_HELPER selects that for us, right?
>>>>>>>>>>
>>>>>>>>>> Hm right I've missed that. Reminds me that you need one more patch at the
>>>>>>>>>> end to remove all the various select DRM_KMS_FB_HELPER from all drm
>>>>>>>>>> drivers. Otherwise this knob here won't work by default if you e.g. select
>>>>>>>>>> radeon. In general we can't mix explicit options with menu entries with a
>>>>>>>>>> select.
>>>>>>>>>
>>>>>>>>> I was trying that out. Removing DRM_KMS_FB_HELPER and having
>>>>>>>>> DRM_FBDEV_EMULATION disabled breaks drivers which use FB stuff internally in
>>>>>>>>> their respective xyz_fbdev.c files.
>>>>>>>>
>>>>>>>> But with the stubbed out functions that should work, right? Why doesn't
>>>>>>>> it?
>>>>>>>
>>>>>>> There are still calls to functions from fb core like fb_set_suspend and
>>>>>>> register_framebuffer which aren't covered by the drm fb helper functions.
>>>>>>
>>>>>> Hm, sounds like we need another patch to stub out fb_set_suspend when
>>>>>> fbdev isn't enabled. Is there anything else?
>>>>>
>>>>> There are a handful of fb core functions which are called by drm drivers:
>>>>>
>>>>> fb_alloc_cmap/fb_dealloc_cmap
>>>>>
>>>>> fb_sys_read/fb_sys_write
>>>>>
>>>>> register_framebuffer/unregister_framebuffer/unlink_framebuffer/
>>>>> remove_conflicting_framebuffers
>>>>>
>>>>> fb_set_suspend
>>>>>
>>>>> fb_deferred_io_init/fb_deferred_io_cleanup
>>>>>
>>>>> framebuffer_alloc/framebuffer_release
>>>>
>>>> Hm yeah that's somewhat annoying indeed. What about the following:
>>>> 1. We move all the #include <linux/fb.h> from drivers into drm_fb_helper.h
>>>>
>>>> 2. Then we add stubs for these functions in drm_fb_helper.h, like this
>>>>
>>>> #if defined(CONFIG_FB)
>>>> #include <linux/fb.h>
>>>> #else
>>>>
>>>> /* static inline stubs for all the fb stuff used by kms drivers */
>>>> #endif
>>>>
>>>> Imo this makes sense since kms drivers really have a bit a special
>>>> situation with fbdev. They're not full-blown fbdev drivers and can be
>>>> useful fully without fbdev.
>>>>
>>>
>>> I was trying this out. Removing 'linux/fb.h' and replacing stub fb funcs
>>> won't really work because struct declarations(like fb_info) also get
>>> removed.
>>>
>>> I considered placing '#if IS_ENABLED(CONFIG_FB)' within linux/fb.h itself,
>>> but that seemed a bit too intrusive.
>>>
>>> This is what I'm currently doing:
>>>
>>> - Some funcs, like framebufer_alloc/release, alloc_cmap/dealloc_cmap would
>>> actually benefit if we have drm fb helpers for them. They are used in
>>> exactly the same manner by all the drivers.
>>>
>>> - For the rest of the functions that are sparsely used, I was considering
>>> making very simple drm_fb_* wrapper functions. Something like:
>>>
>>> void drm_fb_helper_deferred_io_init(struct drm_fb_helper *helper)
>>> {
>>> if (helper->fbdev)
>>> fb_deferred_io_init(helper->fbdev);
>>> }
>>>
>>> We could have all fb calls called within drm_fb_helper.c, creating
>>> drm_fb_helper_* stub functions would then be an easier task. What do you
>>> think?
>>
>> That's actually an option I considered, but I hoped we could do it with
>> less work. If this indeed works and you can create the patch that would be
>> awesome.
>>
>> Thanks, Daniel
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> http://blog.ffwll.ch
>
>
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
Hi,
I think that what have been done by Rob with module_param is also a good idea:
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/msm/msm_drv.c?id=e90dfec78ec288d6c89a7b508a5c5d4ae8b7f934
Can you mix compilation flag and module param ?
Benjamin
2015-06-30 9:56 GMT+02:00 Archit Taneja <[email protected]>:
> Hi,
>
> On 06/30/2015 12:40 PM, Daniel Vetter wrote:
>>
>> Any updates on this or too much distractions? I really think doing
>> this would be awesome for the drm subsystem, instead of reinventing
>> this wheel for each driver.
>
>
> I'd started on this again. I've got more free time now than before, so I
> should have something in a week or so. I agree it will help a lot (there are
> already two new drivers since we started discussing this!)
>
> Archit
>
>
>> -Daniel
>>
>> On Wed, Mar 25, 2015 at 10:21 AM, Daniel Vetter <[email protected]> wrote:
>>>
>>> On Wed, Mar 25, 2015 at 01:47:54PM +0530, Archit Taneja wrote:
>>>>
>>>> Hi,
>>>>
>>>> On 03/13/2015 02:36 PM, Daniel Vetter wrote:
>>>>>
>>>>> On Fri, Mar 13, 2015 at 11:55:07AM +0530, Archit Taneja wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 03/11/2015 08:47 PM, Daniel Vetter wrote:
>>>>>>>
>>>>>>> On Wed, Mar 11, 2015 at 01:51:02PM +0530, Archit Taneja wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 03/10/2015 05:47 PM, Daniel Vetter wrote:
>>>>>>>>>
>>>>>>>>> On Tue, Mar 10, 2015 at 03:52:41PM +0530, Archit Taneja wrote:
>>>>>>>>>>
>>>>>>>>>> On 03/10/2015 03:35 PM, Daniel Vetter wrote:
>>>>>>>>>>>
>>>>>>>>>>> On Tue, Mar 10, 2015 at 03:22:49PM +0530, Archit Taneja wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> On 03/10/2015 03:17 PM, Daniel Vetter wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Tue, Mar 10, 2015 at 03:11:28PM +0530, Archit Taneja wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>>>>>>>>>>>>>> index 151a050..38f83a0 100644
>>>>>>>>>>>>>> --- a/drivers/gpu/drm/Kconfig
>>>>>>>>>>>>>> +++ b/drivers/gpu/drm/Kconfig
>>>>>>>>>>>>>> @@ -40,6 +40,24 @@ config DRM_KMS_FB_HELPER
>>>>>>>>>>>>>> help
>>>>>>>>>>>>>> FBDEV helpers for KMS drivers.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> +config DRM_FBDEV_EMULATION
>>>>>>>>>>>>>> + bool "Enable legacy fbdev support for your modesetting
>>>>>>>>>>>>>> driver"
>>>>>>>>>>>>>> + depends on DRM
>>>>>>>>>>>>>> + select DRM_KMS_HELPER
>>>>>>>>>>>>>> + select DRM_KMS_FB_HELPER
>>>>>>>>>>>>>> + select FB_SYS_FILLRECT
>>>>>>>>>>>>>> + select FB_SYS_COPYAREA
>>>>>>>>>>>>>> + select FB_SYS_IMAGEBLIT
>>>>>>>>>>>>>> + select FB_SYS_FOPS
>>>>>>>>>>>>>> + select FB_CFB_FILLRECT
>>>>>>>>>>>>>> + select FB_CFB_COPYAREA
>>>>>>>>>>>>>> + select FB_CFB_IMAGEBLIT
>>>>>>>>>>>>>> + default y
>>>>>>>>>>>>>> + help
>>>>>>>>>>>>>> + Choose this option if you have a need for the legacy
>>>>>>>>>>>>>> fbdev
>>>>>>>>>>>>>> + support. Note that this support also provide the linux
>>>>>>>>>>>>>> console
>>>>>>>>>>>>>> + support on top of your modesetting driver.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Maybe clarify that for linux console support you also need
>>>>>>>>>>>>> CONFIG_FRAMEBUFFER_CONSOLE? fbdev alone isn't enough.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> DRM_KMS_FB_HELPER selects that for us, right?
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Hm right I've missed that. Reminds me that you need one more
>>>>>>>>>>> patch at the
>>>>>>>>>>> end to remove all the various select DRM_KMS_FB_HELPER from all
>>>>>>>>>>> drm
>>>>>>>>>>> drivers. Otherwise this knob here won't work by default if you
>>>>>>>>>>> e.g. select
>>>>>>>>>>> radeon. In general we can't mix explicit options with menu
>>>>>>>>>>> entries with a
>>>>>>>>>>> select.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> I was trying that out. Removing DRM_KMS_FB_HELPER and having
>>>>>>>>>> DRM_FBDEV_EMULATION disabled breaks drivers which use FB stuff
>>>>>>>>>> internally in
>>>>>>>>>> their respective xyz_fbdev.c files.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> But with the stubbed out functions that should work, right? Why
>>>>>>>>> doesn't
>>>>>>>>> it?
>>>>>>>>
>>>>>>>>
>>>>>>>> There are still calls to functions from fb core like fb_set_suspend
>>>>>>>> and
>>>>>>>> register_framebuffer which aren't covered by the drm fb helper
>>>>>>>> functions.
>>>>>>>
>>>>>>>
>>>>>>> Hm, sounds like we need another patch to stub out fb_set_suspend when
>>>>>>> fbdev isn't enabled. Is there anything else?
>>>>>>
>>>>>>
>>>>>> There are a handful of fb core functions which are called by drm
>>>>>> drivers:
>>>>>>
>>>>>> fb_alloc_cmap/fb_dealloc_cmap
>>>>>>
>>>>>> fb_sys_read/fb_sys_write
>>>>>>
>>>>>> register_framebuffer/unregister_framebuffer/unlink_framebuffer/
>>>>>> remove_conflicting_framebuffers
>>>>>>
>>>>>> fb_set_suspend
>>>>>>
>>>>>> fb_deferred_io_init/fb_deferred_io_cleanup
>>>>>>
>>>>>> framebuffer_alloc/framebuffer_release
>>>>>
>>>>>
>>>>> Hm yeah that's somewhat annoying indeed. What about the following:
>>>>> 1. We move all the #include <linux/fb.h> from drivers into
>>>>> drm_fb_helper.h
>>>>>
>>>>> 2. Then we add stubs for these functions in drm_fb_helper.h, like this
>>>>>
>>>>> #if defined(CONFIG_FB)
>>>>> #include <linux/fb.h>
>>>>> #else
>>>>>
>>>>> /* static inline stubs for all the fb stuff used by kms drivers */
>>>>> #endif
>>>>>
>>>>> Imo this makes sense since kms drivers really have a bit a special
>>>>> situation with fbdev. They're not full-blown fbdev drivers and can be
>>>>> useful fully without fbdev.
>>>>>
>>>>
>>>> I was trying this out. Removing 'linux/fb.h' and replacing stub fb funcs
>>>> won't really work because struct declarations(like fb_info) also get
>>>> removed.
>>>>
>>>> I considered placing '#if IS_ENABLED(CONFIG_FB)' within linux/fb.h
>>>> itself,
>>>> but that seemed a bit too intrusive.
>>>>
>>>> This is what I'm currently doing:
>>>>
>>>> - Some funcs, like framebufer_alloc/release, alloc_cmap/dealloc_cmap
>>>> would
>>>> actually benefit if we have drm fb helpers for them. They are used in
>>>> exactly the same manner by all the drivers.
>>>>
>>>> - For the rest of the functions that are sparsely used, I was
>>>> considering
>>>> making very simple drm_fb_* wrapper functions. Something like:
>>>>
>>>> void drm_fb_helper_deferred_io_init(struct drm_fb_helper *helper)
>>>> {
>>>> if (helper->fbdev)
>>>> fb_deferred_io_init(helper->fbdev);
>>>> }
>>>>
>>>> We could have all fb calls called within drm_fb_helper.c, creating
>>>> drm_fb_helper_* stub functions would then be an easier task. What do you
>>>> think?
>>>
>>>
>>> That's actually an option I considered, but I hoped we could do it with
>>> less work. If this indeed works and you can create the patch that would
>>> be
>>> awesome.
>>>
>>> Thanks, Daniel
>>> --
>>> Daniel Vetter
>>> Software Engineer, Intel Corporation
>>> http://blog.ffwll.ch
>>
>>
>>
>>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
--
Benjamin Gaignard
Graphic Working Group
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Tue, Jun 30, 2015 at 10:31 AM, Benjamin Gaignard
<[email protected]> wrote:
> I think that what have been done by Rob with module_param is also a good idea:
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/msm/msm_drv.c?id=e90dfec78ec288d6c89a7b508a5c5d4ae8b7f934
>
> Can you mix compilation flag and module param ?
Hm, the point of the kconfig is to not require all the legacy baggage
from fbdev. But I guess a module option could be added later on too.
bochs has it too.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
Hi,
On 06/30/2015 02:01 PM, Benjamin Gaignard wrote:
> Hi,
>
> I think that what have been done by Rob with module_param is also a good idea:
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/msm/msm_drv.c?id=e90dfec78ec288d6c89a7b508a5c5d4ae8b7f934
>
> Can you mix compilation flag and module param ?
I guess this won't be hard to do once we have a common fbdev emulation
config option. We could consider this as a drm top level module param.
I'll keep this in mind.
Thanks,
Archit
>
> Benjamin
>
> 2015-06-30 9:56 GMT+02:00 Archit Taneja <[email protected]>:
>> Hi,
>>
>> On 06/30/2015 12:40 PM, Daniel Vetter wrote:
>>>
>>> Any updates on this or too much distractions? I really think doing
>>> this would be awesome for the drm subsystem, instead of reinventing
>>> this wheel for each driver.
>>
>>
>> I'd started on this again. I've got more free time now than before, so I
>> should have something in a week or so. I agree it will help a lot (there are
>> already two new drivers since we started discussing this!)
>>
>> Archit
>>
>>
>>> -Daniel
>>>
>>> On Wed, Mar 25, 2015 at 10:21 AM, Daniel Vetter <[email protected]> wrote:
>>>>
>>>> On Wed, Mar 25, 2015 at 01:47:54PM +0530, Archit Taneja wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> On 03/13/2015 02:36 PM, Daniel Vetter wrote:
>>>>>>
>>>>>> On Fri, Mar 13, 2015 at 11:55:07AM +0530, Archit Taneja wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 03/11/2015 08:47 PM, Daniel Vetter wrote:
>>>>>>>>
>>>>>>>> On Wed, Mar 11, 2015 at 01:51:02PM +0530, Archit Taneja wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 03/10/2015 05:47 PM, Daniel Vetter wrote:
>>>>>>>>>>
>>>>>>>>>> On Tue, Mar 10, 2015 at 03:52:41PM +0530, Archit Taneja wrote:
>>>>>>>>>>>
>>>>>>>>>>> On 03/10/2015 03:35 PM, Daniel Vetter wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> On Tue, Mar 10, 2015 at 03:22:49PM +0530, Archit Taneja wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 03/10/2015 03:17 PM, Daniel Vetter wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Tue, Mar 10, 2015 at 03:11:28PM +0530, Archit Taneja wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>>>>>>>>>>>>>>> index 151a050..38f83a0 100644
>>>>>>>>>>>>>>> --- a/drivers/gpu/drm/Kconfig
>>>>>>>>>>>>>>> +++ b/drivers/gpu/drm/Kconfig
>>>>>>>>>>>>>>> @@ -40,6 +40,24 @@ config DRM_KMS_FB_HELPER
>>>>>>>>>>>>>>> help
>>>>>>>>>>>>>>> FBDEV helpers for KMS drivers.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> +config DRM_FBDEV_EMULATION
>>>>>>>>>>>>>>> + bool "Enable legacy fbdev support for your modesetting
>>>>>>>>>>>>>>> driver"
>>>>>>>>>>>>>>> + depends on DRM
>>>>>>>>>>>>>>> + select DRM_KMS_HELPER
>>>>>>>>>>>>>>> + select DRM_KMS_FB_HELPER
>>>>>>>>>>>>>>> + select FB_SYS_FILLRECT
>>>>>>>>>>>>>>> + select FB_SYS_COPYAREA
>>>>>>>>>>>>>>> + select FB_SYS_IMAGEBLIT
>>>>>>>>>>>>>>> + select FB_SYS_FOPS
>>>>>>>>>>>>>>> + select FB_CFB_FILLRECT
>>>>>>>>>>>>>>> + select FB_CFB_COPYAREA
>>>>>>>>>>>>>>> + select FB_CFB_IMAGEBLIT
>>>>>>>>>>>>>>> + default y
>>>>>>>>>>>>>>> + help
>>>>>>>>>>>>>>> + Choose this option if you have a need for the legacy
>>>>>>>>>>>>>>> fbdev
>>>>>>>>>>>>>>> + support. Note that this support also provide the linux
>>>>>>>>>>>>>>> console
>>>>>>>>>>>>>>> + support on top of your modesetting driver.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Maybe clarify that for linux console support you also need
>>>>>>>>>>>>>> CONFIG_FRAMEBUFFER_CONSOLE? fbdev alone isn't enough.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> DRM_KMS_FB_HELPER selects that for us, right?
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Hm right I've missed that. Reminds me that you need one more
>>>>>>>>>>>> patch at the
>>>>>>>>>>>> end to remove all the various select DRM_KMS_FB_HELPER from all
>>>>>>>>>>>> drm
>>>>>>>>>>>> drivers. Otherwise this knob here won't work by default if you
>>>>>>>>>>>> e.g. select
>>>>>>>>>>>> radeon. In general we can't mix explicit options with menu
>>>>>>>>>>>> entries with a
>>>>>>>>>>>> select.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> I was trying that out. Removing DRM_KMS_FB_HELPER and having
>>>>>>>>>>> DRM_FBDEV_EMULATION disabled breaks drivers which use FB stuff
>>>>>>>>>>> internally in
>>>>>>>>>>> their respective xyz_fbdev.c files.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> But with the stubbed out functions that should work, right? Why
>>>>>>>>>> doesn't
>>>>>>>>>> it?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> There are still calls to functions from fb core like fb_set_suspend
>>>>>>>>> and
>>>>>>>>> register_framebuffer which aren't covered by the drm fb helper
>>>>>>>>> functions.
>>>>>>>>
>>>>>>>>
>>>>>>>> Hm, sounds like we need another patch to stub out fb_set_suspend when
>>>>>>>> fbdev isn't enabled. Is there anything else?
>>>>>>>
>>>>>>>
>>>>>>> There are a handful of fb core functions which are called by drm
>>>>>>> drivers:
>>>>>>>
>>>>>>> fb_alloc_cmap/fb_dealloc_cmap
>>>>>>>
>>>>>>> fb_sys_read/fb_sys_write
>>>>>>>
>>>>>>> register_framebuffer/unregister_framebuffer/unlink_framebuffer/
>>>>>>> remove_conflicting_framebuffers
>>>>>>>
>>>>>>> fb_set_suspend
>>>>>>>
>>>>>>> fb_deferred_io_init/fb_deferred_io_cleanup
>>>>>>>
>>>>>>> framebuffer_alloc/framebuffer_release
>>>>>>
>>>>>>
>>>>>> Hm yeah that's somewhat annoying indeed. What about the following:
>>>>>> 1. We move all the #include <linux/fb.h> from drivers into
>>>>>> drm_fb_helper.h
>>>>>>
>>>>>> 2. Then we add stubs for these functions in drm_fb_helper.h, like this
>>>>>>
>>>>>> #if defined(CONFIG_FB)
>>>>>> #include <linux/fb.h>
>>>>>> #else
>>>>>>
>>>>>> /* static inline stubs for all the fb stuff used by kms drivers */
>>>>>> #endif
>>>>>>
>>>>>> Imo this makes sense since kms drivers really have a bit a special
>>>>>> situation with fbdev. They're not full-blown fbdev drivers and can be
>>>>>> useful fully without fbdev.
>>>>>>
>>>>>
>>>>> I was trying this out. Removing 'linux/fb.h' and replacing stub fb funcs
>>>>> won't really work because struct declarations(like fb_info) also get
>>>>> removed.
>>>>>
>>>>> I considered placing '#if IS_ENABLED(CONFIG_FB)' within linux/fb.h
>>>>> itself,
>>>>> but that seemed a bit too intrusive.
>>>>>
>>>>> This is what I'm currently doing:
>>>>>
>>>>> - Some funcs, like framebufer_alloc/release, alloc_cmap/dealloc_cmap
>>>>> would
>>>>> actually benefit if we have drm fb helpers for them. They are used in
>>>>> exactly the same manner by all the drivers.
>>>>>
>>>>> - For the rest of the functions that are sparsely used, I was
>>>>> considering
>>>>> making very simple drm_fb_* wrapper functions. Something like:
>>>>>
>>>>> void drm_fb_helper_deferred_io_init(struct drm_fb_helper *helper)
>>>>> {
>>>>> if (helper->fbdev)
>>>>> fb_deferred_io_init(helper->fbdev);
>>>>> }
>>>>>
>>>>> We could have all fb calls called within drm_fb_helper.c, creating
>>>>> drm_fb_helper_* stub functions would then be an easier task. What do you
>>>>> think?
>>>>
>>>>
>>>> That's actually an option I considered, but I hoped we could do it with
>>>> less work. If this indeed works and you can create the patch that would
>>>> be
>>>> awesome.
>>>>
>>>> Thanks, Daniel
>>>> --
>>>> Daniel Vetter
>>>> Software Engineer, Intel Corporation
>>>> http://blog.ffwll.ch
>>>
>>>
>>>
>>>
>>
>> --
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>> a Linux Foundation Collaborative Project
>
>
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On Tue, Jun 30, 2015 at 11:04:45AM +0200, Daniel Vetter wrote:
> On Tue, Jun 30, 2015 at 10:31 AM, Benjamin Gaignard
> <[email protected]> wrote:
> > I think that what have been done by Rob with module_param is also a good idea:
> > https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/msm/msm_drv.c?id=e90dfec78ec288d6c89a7b508a5c5d4ae8b7f934
> >
> > Can you mix compilation flag and module param ?
>
> Hm, the point of the kconfig is to not require all the legacy baggage
> from fbdev. But I guess a module option could be added later on too.
> bochs has it too.
I think the option would primarily be useful for testing. I've often
needed to test modesetting code without fbdev getting in the way and the
only way to currently do that is by recompiling with legacy fbdev
disabled. The option would allow an fbdev-enabled kernel to be used but
disable the fbdev code if desired.
Thierry
On Fri, Jul 3, 2015 at 8:32 AM, Thierry Reding <[email protected]> wrote:
> On Tue, Jun 30, 2015 at 11:04:45AM +0200, Daniel Vetter wrote:
>> On Tue, Jun 30, 2015 at 10:31 AM, Benjamin Gaignard
>> <[email protected]> wrote:
>> > I think that what have been done by Rob with module_param is also a good idea:
>> > https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/msm/msm_drv.c?id=e90dfec78ec288d6c89a7b508a5c5d4ae8b7f934
>> >
>> > Can you mix compilation flag and module param ?
>>
>> Hm, the point of the kconfig is to not require all the legacy baggage
>> from fbdev. But I guess a module option could be added later on too.
>> bochs has it too.
>
> I think the option would primarily be useful for testing. I've often
> needed to test modesetting code without fbdev getting in the way and the
> only way to currently do that is by recompiling with legacy fbdev
> disabled. The option would allow an fbdev-enabled kernel to be used but
> disable the fbdev code if desired.
and for debugging.. when things are going wrong it's nice for the
first modeset to not happen under console_lock ;-)
BR,
-R
> Thierry
>
> _______________________________________________
> dri-devel mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>