<div dir="ltr"><div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Mon, Dec 16, 2024 at 4:28 AM Dmitry Baryshkov <<a href="mailto:dmitry.baryshkov@linaro.org">dmitry.baryshkov@linaro.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Mon, Dec 16, 2024 at 12:40:54AM -0500, Marek Olšák wrote:<br>
> git send-email (or rather the way it sends email) has been banned by gmail<br>
> due to being considered unsecure. I don't plan to find a way to make it<br>
> work and I don't plan to use a different email provider. It doesn't matter<br>
> because I'll be the committer of this patch in our amd-staging-drm-next<br>
> branch.<br>
<br>
I'm sorry, but I'd second Joshua. As a community we use certain methods<br>
and certain approaches which makes it easier for other people to review<br>
your patches. One of those includes sending patches in plain text<br>
without any attachments, etc (documented under Documentation/process/).<br>
All my patches are being sent using git-send-email or b4 send via GMail.<br>
Other developers use web-relay provided by the B4 tool.<br>
<br>
Next, the MAINTAINERS file lists Alex, Christian and Xinhui as<br>
maintainers of the drm/amd tree. If the file is incorrect or incomplete,<br>
please correct it.<br>
<br>
Last, but not least, this patch will likely go into drm-misc-next or<br>
drm-next instead of amd-saging-drm-next. It is not AMD-specific.<br></blockquote><div><br></div><div>I won't comment on this because it's irrelevant. Alex will decide which pull request it will end up in.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> Let's talk about the concept and brokenness of DRM_FORMAT_MOD_LINEAR, not<br>
> send-email.<br>
<br>
The biggest problem with your approach is tht it is not clear which<br>
modifier to use. For example, if one of the formats requires 128-byte<br>
alignment, should the driver list just 128B or both 128B and 256B? If at<br>
some point we add 32B alignment, will we have to update the drivers?<br>
Which format should be used for exported buffers? Please provide<br>
corresponding driver patches that utilize new uAPI.<br></blockquote><div><br></div><div>This is format(bpp)-independent. If some formats don't support some alignment, only modifiers that are supported by all formats should be exposed.</div><div><br></div><div>Drivers should list the alignment they support and all greater ones, e.g.:</div><div>Intel: 64B, 128B, 256B<br></div><div>AMD: 256B</div><div><br></div><div>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.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Also, please don't forget the backwards compatibility issue. If we<br>
follow this approach, the drivers have to list both LINEAR and new<br>
PITCH_ALIGN modifiers. So the userspace still will attempt to use<br>
LINEAR.<br></blockquote><div><br></div><div>Yes and no. Apps should never get an image using LINEAR if PITCH_ALIGN is first in the list.<br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
It is true that such requirements are platform-specific and are usually<br>
encoded in the compostitor. I think it might make more sense to export<br>
the pitch requirements using the extra hint-like property, which then<br>
can be used by a smart userspace.<br></blockquote><div><br></div><div>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.<br></div><div> </div>Marek<br></div></div>