Re: [Intel-gfx] [PATCH v5 5/7] drm/i915: Initialize fbdev DRM client with callback functions

[Thomas Zimmermann <tzimmermann@suse.de>]

[-- Attachment #1.1: Type: text/plain, Size: 3454 bytes --]

Hi

Am 01.11.23 um 10:10 schrieb Hogander, Jouni:
[...]
>>> We are currently working on new driver named as Xe. Due to this it
>>
>> I've always thought that it's an entirely new driver. But I'm not
>> really
>> up-to-date. So the Xe driver is located under i915/ and also shares
>> code
>> with the existing i915 driver?
> 
> It will mainly share display code with i915.
> 
>>
>>> might actually make sense to use intel-fbdev here rather than i915-
>>> fbdev.
>>
>> That change could break user-space programs. See the comment at [1]
>> and
>> the commit 842470c4e211 ("Revert "drm/fb-helper: improve DRM fbdev
>> emulation device names"").  I'd rather leave the string as it is.
> 
> Client name doesn't affect name used in fbinfo. For i915 it is
> i915drmfb as earlier and for xe its xedrmfb. Also this client name is
> completely new added by your patches. I'm not sure where it will
> visible. I was originally thinking using driver->name as done in
> drm_fb_helper_fill_info, but I think common intel-fbdev is just fine.

Right, I was confusing the names. The client name is the name of the 
framebuffer in various places.

https://elixir.bootlin.com/linux/v6.6/source/drivers/gpu/drm/drm_client.c#L421
https://elixir.bootlin.com/linux/v6.6/source/drivers/gpu/drm/drm_framebuffer.c#L1182

But it appears to be overwritten by fbdev later during initialization:

https://elixir.bootlin.com/linux/v6.6/source/drivers/gpu/drm/drm_fb_helper.c#L1672

I'll change the name then, but I'm not sure if it'll make a difference 
in the end.

Best regards
Thomas

> 
> BR,
> 
> Jouni Högander
>   
>>
>> Best regards
>> Thomas
>>
>> [1]
>> https://elixir.bootlin.com/linux/v6.6/source/drivers/gpu/drm/drm_fb_helper.c#L1755
>>
>>>
>>> BR,
>>>
>>> Jouni Högander
>>>
>>>> +                             &intel_fbdev_client_funcs);
>>>> +       if (ret)
>>>> +               goto err_drm_fb_helper_unprepare;
>>>> +
>>>>           ret = drm_fb_helper_init(dev, &ifbdev->helper);
>>>> -       if (ret) {
>>>> -               kfree(ifbdev);
>>>> -               return ret;
>>>> -       }
>>>> +       if (ret)
>>>> +               goto err_drm_client_release;
>>>>    
>>>>           dev_priv->display.fbdev.fbdev = ifbdev;
>>>>           INIT_WORK(&dev_priv->display.fbdev.suspend_work,
>>>> intel_fbdev_suspend_worker);
>>>>    
>>>>           return 0;
>>>> +
>>>> +err_drm_client_release:
>>>> +       drm_client_release(&ifbdev->helper.client);
>>>> +err_drm_fb_helper_unprepare:
>>>> +       drm_fb_helper_unprepare(&ifbdev->helper);
>>>> +       kfree(ifbdev);
>>>> +       return ret;
>>>>    }
>>>>    
>>>>    static void intel_fbdev_initial_config(void *data,
>>>> async_cookie_t
>>>> cookie)
>>>
>>
>> -- 
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Frankenstrasse 146, 90461 Nuernberg, Germany
>> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
>> HRB 36809 (AG Nuernberg)
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]