Re: [PATCH 3/4] drm/fb-helper: Add module option to disable fbdev emulation

[Archit Taneja <architt@codeaurora.org>]

On 08/26/2015 10:42 AM, Archit Taneja wrote:
>
>
> On 08/25/2015 07:15 PM, Daniel Vetter wrote:
>> Faster than recompiling.
>>
>> Note that restore_fbdev_mode_unlocked is a bit special and the only
>> one which returns an error code when fbdev isn't there - i915 needs
>> that one to not fall over with some additional fbcon related restore
>> code. Everyone else just ignores the return value or only prints a
>> DRM_DEBUG level message.
>
> Reviewed-by:Archit Taneja <architt@codeaurora.org>


With the module param, and the drivers should see the following state(
based on the truth table below):

module param | config option
    true      |    true       -> real helper funcs called, driver
                                 allocated drm_fb_helper is correctly
                                 populated.

    false     |    true       -> real helper funcs called, but bailed
                                 out early, driver allocated
                                 drm_fb_helper is non-NULL, but we won't
                                 end up using it.

     X        |    false      -> stub functions called, drm_fb_helper is
                                 NULL

I wonder if the 2nd combination could leave space for errors in other
drivers. Drivers tend to check whether the fb_helper struct is NULL
or not, and make decisions based on that. If the decisions are to
just call a drm_fb_helper function, then we're okay. If they do 
something more than that, then we could have an issue.

I'll prepare the remainder of 'Step 2' part of the series over this,
and we can test to check if we don't hit any corner cases.

Archit

>
>>
>> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> ---
>>   drivers/gpu/drm/drm_fb_helper.c | 29 +++++++++++++++++++++++++++++
>>   1 file changed, 29 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c
>> b/drivers/gpu/drm/drm_fb_helper.c
>> index 83aacb0cc9df..8259dec1de1f 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -39,6 +39,11 @@
>>   #include <drm/drm_fb_helper.h>
>>   #include <drm/drm_crtc_helper.h>
>>
>> +static bool drm_fbdev_emulation = true;
>> +module_param_named(fbdev_emulation, drm_fbdev_emulation, bool, 0600);
>> +MODULE_PARM_DESC(fbdev_emulation,
>> +         "Enable legacy fbdev emulation [default=true]");
>> +
>>   static LIST_HEAD(kernel_fb_helper_list);
>>
>>   /**
>> @@ -99,6 +104,9 @@ int drm_fb_helper_single_add_all_connectors(struct
>> drm_fb_helper *fb_helper)
>>       struct drm_connector *connector;
>>       int i;
>>
>> +    if (!drm_fbdev_emulation)
>> +        return 0;
>> +
>>       mutex_lock(&dev->mode_config.mutex);
>>       drm_for_each_connector(connector, dev) {
>>           struct drm_fb_helper_connector *fb_helper_connector;
>> @@ -129,6 +137,9 @@ int drm_fb_helper_add_one_connector(struct
>> drm_fb_helper *fb_helper, struct drm_
>>       struct drm_fb_helper_connector **temp;
>>       struct drm_fb_helper_connector *fb_helper_connector;
>>
>> +    if (!drm_fbdev_emulation)
>> +        return 0;
>> +
>>       WARN_ON(!mutex_is_locked(&fb_helper->dev->mode_config.mutex));
>>       if (fb_helper->connector_count + 1 >
>> fb_helper->connector_info_alloc_count) {
>>           temp = krealloc(fb_helper->connector_info, sizeof(struct
>> drm_fb_helper_connector *) * (fb_helper->connector_count + 1),
>> GFP_KERNEL);
>> @@ -184,6 +195,9 @@ int drm_fb_helper_remove_one_connector(struct
>> drm_fb_helper *fb_helper,
>>       struct drm_fb_helper_connector *fb_helper_connector;
>>       int i, j;
>>
>> +    if (!drm_fbdev_emulation)
>> +        return 0;
>> +
>>       WARN_ON(!mutex_is_locked(&fb_helper->dev->mode_config.mutex));
>>
>>       for (i = 0; i < fb_helper->connector_count; i++) {
>> @@ -375,6 +389,9 @@ int
>> drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper
>> *fb_helper)
>>       bool do_delayed
>>       int ret;
>>
>> +    if (!drm_fbdev_emulation)
>> +        return -ENODEV;
>> +
>>       drm_modeset_lock_all(dev);
>>       ret = restore_fbdev_mode(fb_helper);
>>
>> @@ -591,6 +608,9 @@ int drm_fb_helper_init(struct drm_device *dev,
>>       struct drm_crtc *crtc;
>>       int i;
>>
>> +    if (!drm_fbdev_emulation)
>> +        return 0;
>> +
>>       if (!max_conn_count)
>>           return -EINVAL;
>>
>> @@ -713,6 +733,9 @@ EXPORT_SYMBOL(drm_fb_helper_release_fbi);
>>
>>   void drm_fb_helper_fini(struct drm_fb_helper *fb_helper)
>>   {
>> +    if (!drm_fbdev_emulation)
>> +        return;
>> +
>>       if (!list_empty(&fb_helper->kernel_fb_list)) {
>>           list_del(&fb_helper->kernel_fb_list);
>>           if (list_empty(&kernel_fb_helper_list)) {
>> @@ -1933,6 +1956,9 @@ int drm_fb_helper_initial_config(struct
>> drm_fb_helper *fb_helper, int bpp_sel)
>>       struct drm_device *dev = fb_helper->dev;
>>       int count = 0;
>>
>> +    if (!drm_fbdev_emulation)
>> +        return 0;
>> +
>>       mutex_lock(&dev->mode_config.mutex);
>>       count = drm_fb_helper_probe_connector_modes(fb_helper,
>>                               dev->mode_config.max_width,
>> @@ -1976,6 +2002,9 @@ int drm_fb_helper_hotplug_event(struct
>> drm_fb_helper *fb_helper)
>>       struct drm_device *dev = fb_helper->dev;
>>       u32 max_width, max_height;
>>
>> +    if (!drm_fbdev_emulation)
>> +        return 0;
>> +
>>       mutex_lock(&fb_helper->dev->mode_config.mutex);
>>       if (!fb_helper->fb || !drm_fb_helper_is_bound(fb_helper)) {
>>           fb_helper->delayed_hotplug = true;
>>
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx