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

[Thomas Zimmermann <tzimmermann@suse.de>]

Hi


Am 15.01.25 um 11:58 schrieb Tomi Valkeinen:
[...]
>> These are all good points. Did you read my discussion with Andy on 
>> patch 2? I think it resolves all the points you have. The current 
>> CREATE_DUMB 
>
> I had missed the discussion, and, indeed, the patch you attached fixes 
> the problem on Xilinx.

Great. Thanks for testing.

>
>> ioctl is unsuited for anything but the simple RGB formats. The bpp 
>
> It's a bit difficult to use, but is it really unsuited? bitsperpixel, 
> width and height do give an exact pitch and size, do they not? It does 
> require the userspace to handle the subsampling and planes, though, so 
> far from perfect.

The bpp value sets the number of bits per pixel; except for bpp==15 
(XRGB1555), where it sets the color depth. OR bpp is the color depth; 
except for bpp==32 (XRGB8888), where it is the number of bits per pixel. 
It's therefore best to interpret it like a color-mode enum.

>
> So, I'm all for a new ioctl, but I don't right away see why the 
> current ioctl couldn't be used. Which makes me wonder about the 
> drm_warn() in your patch, and the "userspace throws in arbitrary 
> values for bpp and relies on the kernel to figure it out". Maybe I'm 
> missing something here.

I was unsure about the drm_warn() as well. It's not really wrong to have 
odd bpp values, but handing in an unknown bpp value might point to a 
user-space error. At least there should be a drm_dbg().

>
>> parameter is not very precise. The solution would be a new ioctl call 
>> that receives the DRM format and returns a buffer for each individual 
>> plane.
>
> Yes, I think that makes sense. That's a long road, though =). So my 
> question is, is CREATE_DUMB really unsuitable for other than simple 
> RGB formats, or can it be suitable if we just define how the userspace 
> should use it for multiplanar, subsampled formats?

That would duplicate format and hardware information in user-space. Some 
hardware might have odd per-plane limitations that only the driver knows 
about. For example, there's another discussion on dri-devel about 
pitch-alignment requirements of DRM_FORMAT_MOD_LINEAR on various 
hardware. That affects dumb buffers as well. I don't think that there's 
an immediate need for a CREATE_DUMB2, but it seems worth to keep in mind.

Best regards
Thomas

>
>  Tomi
>

-- 
--
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)