[PATCH 1/4] drm/udl: Replace fbdev code with generic emulation

[Thomas Zimmermann]

Hi

Am 12.11.19 um 14:40 schrieb Daniel Vetter:
> On Tue, Nov 12, 2019 at 12:55 PM Thomas Zimmermann <tzimmermann at suse.de> wrote:
>>
>> Hi
>>
>> Am 08.11.19 um 16:37 schrieb Noralf Trønnes:
>>>
>>>
>>> Den 08.11.2019 13.33, skrev Thomas Zimmermann:
>>>> The udl driver can use the generic fbdev implementation. Convert it.
>>>>
>>>> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
>>>> ---
>>>
>>>> diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
>>>> index 563cc5809e56..55c0f9dfee29 100644
>>>> --- a/drivers/gpu/drm/udl/udl_drv.c
>>>> +++ b/drivers/gpu/drm/udl/udl_drv.c
>>>
>>>> @@ -47,6 +48,8 @@ static struct drm_driver driver = {
>>>>      .driver_features = DRIVER_MODESET | DRIVER_GEM,
>>>>      .release = udl_driver_release,
>>>>
>>>> +    .lastclose = drm_fb_helper_lastclose,
>>>> +
>>>
>>> No need to set this, it's already wired up:
>>>
>>> drm_lastclose -> drm_client_dev_restore -> drm_fbdev_client_restore ->
>>> drm_fb_helper_lastclose
>>>
>>>>      /* gem hooks */
>>>>      .gem_create_object = udl_driver_gem_create_object,
>>>>
>>>
>>>> diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
>>>> index f8153b726343..afe74f892a2b 100644
>>>> --- a/drivers/gpu/drm/udl/udl_fb.c
>>>> +++ b/drivers/gpu/drm/udl/udl_fb.c
>>>> @@ -20,19 +20,9 @@
>>>>
>>>>  #include "udl_drv.h"
>>>>
>>>> -#define DL_DEFIO_WRITE_DELAY    (HZ/20) /* fb_deferred_io.delay in jiffies */
>>>> -
>>>> -static int fb_defio = 0;  /* Optionally enable experimental fb_defio mmap support */
>>>>  static int fb_bpp = 16;
>>>>
>>>>  module_param(fb_bpp, int, S_IWUSR | S_IRUSR | S_IWGRP | S_IRGRP);
>>>
>>> Maybe fb_bpp can be dropped too?
>>
>> Sure, makes sense.
>>
>> The driver exposes a preferred color depth of 24 bpp, which we may want
>> to change to 16 then. The internal framebuffer is only 16 bpp anyway.
> 
> Just something that crossed my mind: Should we ensure that the
> preferred format of the primary plane (should be the first in the
> format array) matches up with the preferred bpp setting? Maybe even
> enforce that for drivers with an explicit primary plane (i.e. atomic
> drivers). I think tiny drivers get this right already.

IMHO that makes if the userspace can handle it. The preferred bpp could
also be retrieved from the formats array automatically. What about HW
with multiple CRTCs with different format defaults (sounds weird, I know)?

WRT udl: For v3 of this patchset I've set the preferred color depth to
32 bpp; although the internal FB is always at 16 bpp. Because when I
tested with a dual-screen setup (radeon + udl) X11 didn't support the 16
bpp output on the second screen (the one driven by udl). Only setting
both screen to 32 bpp worked out of the box. And the preferred 24 bpp
are not even supported by udl.

Best regards
Thomas

> -Daniel
> 
>>
>> Best regards
>> Thomas
>>
>>>
>>> It's possible to set it on the command line:
>>>
>>> video=<xres>x<yres>-<bpp>
>>>
>>> I haven't tried it so I can't say for certain that it actually works>
>>> Ref: Documentation/fb/modedb.rst and drm_fb_helper_single_fb_probe()
>>>
>>>> -module_param(fb_defio, int, S_IWUSR | S_IRUSR | S_IWGRP | S_IRGRP);
>>>> -
>>>
>>>> diff --git a/drivers/gpu/drm/udl/udl_modeset.c b/drivers/gpu/drm/udl/udl_modeset.c
>>>> index bc1ab6060dc6..1517d5e881b8 100644
>>>> --- a/drivers/gpu/drm/udl/udl_modeset.c
>>>> +++ b/drivers/gpu/drm/udl/udl_modeset.c
>>>
>>>> @@ -422,7 +423,7 @@ static int udl_crtc_init(struct drm_device *dev)
>>>>
>>>>  static const struct drm_mode_config_funcs udl_mode_funcs = {
>>>>      .fb_create = udl_fb_user_fb_create,
>>>> -    .output_poll_changed = NULL,
>>>> +    .output_poll_changed = drm_fb_helper_output_poll_changed,
>>>
>>> No need to set this, it's already wired up:
>>>
>>> drm_kms_helper_hotplug_event -> drm_client_dev_hotplug ->
>>> drm_fbdev_client_hotplug -> drm_fb_helper_hotplug_event
>>>
>>> Noralf.
>>>
>>>>  };
>>>>
>>>>  int udl_modeset_init(struct drm_device *dev)
>>>>
>>
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>> (HRB 36809, AG Nürnberg)
>> Geschäftsführer: Felix Imendörffer
>>
> 
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20191112/300bccf1/attachment-0001.sig>