Re: [PATCH v4 01/10] drm/fourcc: Add AFBC yuv fourccs for Mali

[Ayan Halder <Ayan.Halder@arm.com>]

On Mon, Mar 18, 2019 at 07:12:24PM +0100, Maarten Lankhorst wrote:
> Op 18-03-2019 om 16:40 schreef Brian Starkey:
> > Hi,
> >
> > On Mon, Mar 18, 2019 at 11:17:55AM +0100, Maarten Lankhorst wrote:
> >
> > <snip>
> >
> >> Hey..
> >>
> >> There's a conflict with this patch and the merge of topic/hdr-formats, resulting in double definitions for Y210, Y410 and P010.
> >>
> >> Worse still is that one has set has_alpha to true for Y41x and other to false.
> >>
> >> ~Maarten
> >>
> > Oh that's sad :-( I think this fell through the cracks on our side
> > when someone left our team. Also turns out I'm not subscribed to
> > igt-dev.
> >
> > I see you commented the same on one of the previous patches, and that
> > there was some discussion of this on the test patches too.
> >
> > I have been referring to Microsoft's page[1] as "the" source for these
> > formats, which does indeed call out Y410 as having 2 bits of alpha.
> > Our GPU expects alpha.
> 
> Ah. Yeah there has been discussion on whether there was supposed to be alpha or not, but the original discussion on HDR formats has been completely ignored by arm.
> 
> The patch had originally a few arm devs on cc and was sent to dri-devel with linux-media cc'd. Was sad to see it completely ignored so after having been sent twice I pushed it.
Apologies, I see that I was cc-ed in the mail 'drm: Add Y2xx and Y4xx
(xx:10/12/16) format definitions and fourcc' sent by
swati2.sharma@intel.com. It got lost in my pile of unread mails. :(

About this patch, I had tagged you in irc channel
(https://people.freedesktop.org/~cbrill/dri-log/?channel=dri-devel&highlight_names=&date=2019-03-11&show_html=true)
for reviewing this seies. Did not hear back from you then ?
> 
> > Was there a specific reason for opting to change the test instead of
> > the definition? Any way to get this changed now?
> >
> > It doesn't seem that sensible for the kernel to call something Y410
> > which doesn't match an "existing" definition by the same name. If
> > alpha needs to be ignored on scanout, the alpha blend mode property
> > can be used (more archaeology - I see that was still giving CRC
> > failures, but that might be a "known issue" for all YUV on your HW?)
> 
> Were a few bugs, but should be fixed now. :)
> 
> Well only that we didn't have hw supporting alpha, and didn't hear back from others so we went without alpha.
In light of the suggestions made by brian.starkey@arm.com, I think
changing the format from Y410 to X410 (in your case) might make sense
as the alpha bits are absent.
If this suggestion looks reasonable to you, I can volunteer myself to make
this change in topic/hdr-formats.
> 
> > -Brian
> >
> > [1] https://docs.microsoft.com/en-us/windows/desktop/medfound/10-bit-and-16-bit-yuv-video-formats#444-formats
>