Hello Laurent,
On 5/2/22 13:34, Laurent Pinchart wrote:
> Hi Javier,
>
> Thank you for the patch.
>
Thanks a lot for your feedback.
[snip]
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -2501,8 +2501,16 @@ static const struct drm_client_funcs drm_fbdev_client_funcs = {
>> /**
>> * drm_fbdev_generic_setup() - Setup generic fbdev emulation
>> * @dev: DRM device
>> - * @preferred_bpp: Preferred bits per pixel for the device.
>> - * @dev->mode_config.preferred_depth is used if this is zero.
>> + * @options: options for the registered framebuffer.
>> + *
>> + * The @options parameter is a multi-field parameter that can contain
>> + * different options for the emulated framebuffer device registered.
>> + *
>> + * The options must be set using DRM_FB_SET_OPTION() and obtained using
>> + * DRM_FB_GET_OPTION(). The options field are the following:
>> + *
>> + * * DRM_FB_BPP: bits per pixel for the device. If the field is not set,
>> + * @dev->mode_config.preferred_depth is used instead.
>
> Do I assume correctly that a driver that would need to set multiple
> options would do something like
>
> drm_fbdev_generic_setup(dev, DRM_FB_SET_OPTION(DRM_FB_BPP, 32) |
> DRM_FB_SET_OPTION(DRM_FB_FW, 1));
>
That's correct, yes.
> ? If so, I would rename DRM_FB_SET_OPTION() to DRM_FB_OPTION() as it's
> computing the value of the option bitfield, it doesn't actually set it.
> Apart from that,
>
Right. I'll rename it.
> Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
>
Thanks!
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat