Re: [PATCH] drm: msm: Fix build when legacy fbdev support isn't set

[Rob Clark]

On Mon, Mar 9, 2015 at 4:54 AM, Archit Taneja <architt@xxxxxxxxxxxxxx> wrote:
>
>
> On 03/09/2015 01:14 PM, Daniel Vetter wrote:
>>
>> On Mon, Mar 09, 2015 at 11:27:06AM +0530, Archit Taneja wrote:
>>>
>>> On 03/05/2015 09:14 PM, Daniel Vetter wrote:
>>>>
>>>> On Thu, Mar 05, 2015 at 07:10:44AM -0500, Rob Clark wrote:
>>>>>
>>>>> On Thu, Mar 5, 2015 at 5:06 AM, Archit Taneja <architt@xxxxxxxxxxxxxx>
>>>>> wrote:
>>>>>>
>>>>>>
>>>>>> On 02/23/2015 09:09 PM, Daniel Vetter wrote:
>>>>>>>
>>>>>>>
>>>>>>> On Mon, Feb 23, 2015 at 10:03:21AM -0500, Rob Clark wrote:
>>>>>>
>>>>>> Rather than creating fb helper stub functions, maybe we could help
>>>>>> each drm
>>>>>> driver create two variants of functions needed by drm core(like
>>>>>> output_poll_changed and dev_lastclose), one variant supporting legacy
>>>>>> fbdev,
>>>>>> and the other not?
>>>>>
>>>>>
>>>>> So one quick thought.. building without fbdev would ideally/eventually
>>>>> be a distro level decision, not a driver level decision..  so I think
>>>>> it is *eventually* not a problem for it being a global drm level
>>>>> decision.  The only problem is right now some drivers support no-fbdev
>>>>> config, and some do not.  Maybe it is worth fixing that?
>>>>
>>>>
>>>> Yeah, if we get fbdev emulation Kconfig option then I think i915 and msm
>>>> should remove their own options and just use that. There's really no
>>>> need
>>>> to have this per-driver, this is a question of what userspace expects
>>>> and
>>>> so per-distro, independant of the driver.
>>>> -Daniel
>>>>
>>>
>>> Okay. If I understand right. We need to do something like this:
>>>
>>> - Create a new Kconfig option that lets us emulate fbdev
>>>
>>> - For drivers that already have a config for fbdev emulation, replace it
>>> with our new emulation config.
>>>
>>> - For drivers that assume fbdev emulation by default, select our new
>>> emulation config in their respective Kconfigs.
>>>
>>> Does this sound okay?
>>>
>>> I suppose this could be the first step. Later we'd need to work on each
>>> driver to work with and without the fbdev emulation Kconfig option.
>>
>>
>> See Rob's idea quoted above: Imo you should just do the conditional code
>> in the fbdev emulation helper in drm_fb_helper.c, not in drivers. There's
>> some additional code in i915 (and maybe also msm) to compile out (mostly
>> around taking/dropping console_lock) and we want to keep that. But that
>> should also just be using the same new config option. So:
>>
>> - Add new config option DRM_FBDEV_EMULATION (default y for backwards
>>    compat). Use that to stub out helpers in drm_fb_helper.c.
>>
>> - Replace the msm/i915 specific options with the new, i.e. remove it from
>>    Kconfig and use DRM_FBDEV_EMULATION in the code instead of the msm/i915
>>    specific one.
>>
>> fbdev-less support for all other drivers will just magically happen
>> without the need for driver-specific code (except for the special locking
>> in i915 and similar things maybe).
>
>
> That sounds good. I wasn't totally sure about fbdev-less support working out
> of the box on devices that use the fb CMA helpers. The drm_fbdev_cma_* funcs
> internally use the fb helper functions. Stubbing the fb helpers out might
> result in some corner cases.
>
> However, after a quick look at that the cma helpers, it looks like it take
> all the necessary precautions to prevent uninitialized accesses. Although,
> it would be ideal to stub out the drm_fbdev_cma_* helper functions too since
> they don't serve any purpose when fbdev emulation is disabled.
>

I think for a first pass, I wouldn't worry too much about a little
unused code in cma fbdev helpers.  If we were going for drm
tinyification I suspect there are bigger fish to fry ;-)

BR,
-R

>
> Archit
>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/