On 15/01/2025 15:45, Thomas Zimmermann wrote:
Hi
Am 15.01.25 um 14:33 schrieb Tomi Valkeinen:
[...]
Yeah, there are constrains in the scanline and buffer alignments and
orientation. And if we say that bpp==12 means NV12, it will be a
problem for all other cases where bpp==12 makes sense.
I feel I still don't quite understand. Can't we define and document
CREATE_DUMB like this:
If (bpp < 8 || is_power_of_two(bpp))
bpp means bitsperpixel
pitch is args->width * args->bpp / 8, aligned up to driver-
specific-align
else
bpp is a legacy parameter, and we deal with it case by case.
list the cases and what they mean
And describe that when allocating subsampled buffers, the caller must
adjust the width and height accordingly. And that the bpp and width
can also refer to pixel groups.
Or if the currently existing code prevents the above for 16 and 32
bpps, how about defining that any non-RGB or not-simple buffer has to
be allocated with bpp=8, and the userspace has to align the pitch
correctly according to the format and platform's hw restrictions?
What if a hardware requires certain per-format alignments? Or requires
certain alignments for each plane? Or only supports tile modes? Or has
strict limits on the maximum buffer size?
It is not possible to encode all this in a simple 32-bit value. So user-
space code has to be aware of all this and tweak bpp-based allocation to
make it work. Obviously you can use the current UAPI for your use case.
It's just not optimal or future proof.
No disagreement there, we need CREATE_DUMB2.
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.
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?
Tomi
