On Mon, Dec 16, 2024 at 4:28 AM Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> wrote:
On Mon, Dec 16, 2024 at 12:40:54AM -0500, Marek Olšák wrote:
> git send-email (or rather the way it sends email) has been banned by gmail
> due to being considered unsecure. I don't plan to find a way to make it
> work and I don't plan to use a different email provider. It doesn't matter
> because I'll be the committer of this patch in our amd-staging-drm-next
> branch.
I'm sorry, but I'd second Joshua. As a community we use certain methods
and certain approaches which makes it easier for other people to review
your patches. One of those includes sending patches in plain text
without any attachments, etc (documented under Documentation/process/).
All my patches are being sent using git-send-email or b4 send via GMail.
Other developers use web-relay provided by the B4 tool.
Next, the MAINTAINERS file lists Alex, Christian and Xinhui as
maintainers of the drm/amd tree. If the file is incorrect or incomplete,
please correct it.
Last, but not least, this patch will likely go into drm-misc-next or
drm-next instead of amd-saging-drm-next. It is not AMD-specific.
I won't comment on this because it's irrelevant. Alex will decide which pull request it will end up in.
> Let's talk about the concept and brokenness of DRM_FORMAT_MOD_LINEAR, not
> send-email.
The biggest problem with your approach is tht it is not clear which
modifier to use. For example, if one of the formats requires 128-byte
alignment, should the driver list just 128B or both 128B and 256B? If at
some point we add 32B alignment, will we have to update the drivers?
Which format should be used for exported buffers? Please provide
corresponding driver patches that utilize new uAPI.
This is format(bpp)-independent. If some formats don't support some alignment, only modifiers that are supported by all formats should be exposed.
Drivers should list the alignment they support and all greater ones, e.g.:
Intel: 64B, 128B, 256B
AMD: 256B
Nobody chooses exactly which modifier to use in advance, and if some app did that, it would be a violation of how modifiers work. Drivers only expose modifiers sorted from best to worst, apps only compute intersections of modifier lists and pass them to drivers, and drivers pick the first one in the list or the best one in the list, but it doesn't matter which one at that point. The computation of the intersection is what determines which modifiers are allowed.
Also, please don't forget the backwards compatibility issue. If we
follow this approach, the drivers have to list both LINEAR and new
PITCH_ALIGN modifiers. So the userspace still will attempt to use
LINEAR.
Yes and no. Apps should never get an image using LINEAR if PITCH_ALIGN is first in the list.
It is true that such requirements are platform-specific and are usually
encoded in the compostitor. I think it might make more sense to export
the pitch requirements using the extra hint-like property, which then
can be used by a smart userspace.
If we did that, it would be an admission that using modifiers exposed by drivers can fail image allocation for any reason, and thus it would be an indication that modifiers are a badly designed API because driver-exposed modifiers don't fully describe image layouts, which is what they were supposed to do in the first place.
