[PATCH v2 25/25] drm/xlnx: Compute dumb-buffer sizes with drm_mode_size_dumb()

[Tomi Valkeinen]

Hi,

On 16/01/2025 12:17, Geert Uytterhoeven wrote:
> On Thu, Jan 16, 2025 at 11:03 AM Tomi Valkeinen
> <tomi.valkeinen at ideasonboard.com> wrote:
>> On 16/01/2025 10:09, Thomas Zimmermann wrote:
>>> Am 15.01.25 um 15:20 schrieb Tomi Valkeinen:
>>> [...]
>>>>
>>>> My point is that we have the current UAPI, and we have userspace using
>>>> it, but we don't have clear rules what the ioctl does with specific
>>>> parameters, and we don't document how it has to be used.
>>>>
>>>> Perhaps the situation is bad, and all we can really say is that
>>>> CREATE_DUMB only works for use with simple RGB formats, and the
>>>> behavior for all other formats is platform specific. But I think even
>>>> that would be valuable in the UAPI docs.
>>>
>>> To be honest, I would not want to specify behavior for anything but the
>>> linear RGB formats. If anything, I'd take Daniel's reply mail for
>>> documentation as-is. Anyone stretching the UAPI beyond RGB is on their own.
>>>
>>>> Thinking about this, I wonder if this change is good for omapdrm or
>>>> xilinx (probably other platforms too that support non-simple non-RGB
>>>> formats via dumb buffers): without this patch, in both drivers, the
>>>> pitch calculations just take the bpp as bit-per-pixels, align it up,
>>>> and that's it.
>>>>
>>>> With this patch we end up using drm_driver_color_mode_format(), and
>>>> aligning buffers according to RGB formats figured out via heuristics.
>>>> It does happen to work, for the formats I tested, but it sounds like
>>>> something that might easily not work, as it's doing adjustments based
>>>> on wrong format.
>>>>
>>>> Should we have another version of drm_mode_size_dumb() which just
>>>> calculates using the bpp, without the drm_driver_color_mode_format()
>>>> path? Or does the drm_driver_color_mode_format() path provide some
>>>> value for the drivers that do not currently do anything similar?
>>>
>>> With the RGB-only rule, using drm_driver_color_mode_format() makes
>>> sense. It aligns dumb buffers and video=, provides error checking, and
>>> overall harmonizes code. The fallback is only required because of the
>>> existing odd cases that already bend the UAPI's rules.
>>
>> I have to disagree here.
>>
>> On the platforms I have been using (omap, tidss, xilinx, rcar) the dumb
>> buffers are the only buffers you can get from the DRM driver. The dumb
>> buffers have been used to allocate linear and multiplanar YUV buffers
>> for a very long time on those platforms.
>>
>> I tried to look around, but I did not find any mentions that CREATE_DUMB
>> should only be used for RGB buffers. Is anyone outside the core
>> developers even aware of it?
>>
>> If we don't use dumb buffers there, where do we get the buffers? Maybe
>> from a v4l2 device or from a gpu device, but often you don't have those.
>> DMA_HEAP is there, of course.
> 
> Why can't there be a variant that takes a proper fourcc format instead of
> an imprecise bpp value?

There can, but it's somewhat a different topic, although it's been 
covered a bit in this thread.

My specific concern here is the current CREATE_DUMB, and (not) changing 
how it behaves.

  Tomi