Messages in this thread | | | From | Brian Starkey <> | Subject | Re: [PATCH v4 01/10] drm/fourcc: Add AFBC yuv fourccs for Mali | Date | Mon, 18 Mar 2019 19:06:39 +0000 |
| |
On Mon, Mar 18, 2019 at 07:00:57PM +0000, Brian Starkey wrote: > 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. > > That's the kernel patch(es)? It looks like I did receive them via > dri-devel, and Ayan was Cc'd on two of the series', but it's > disingenuous to say they had "a few Arm devs". > > I first submitted this patch with Y410 to dri-devel back in August, > and since then it's been sent 8 times by my count (with you on Cc on > all of them!), and all have been similarly completely ignored; so I'm > sorry but I don't think you can put the blame entirely with Arm here. > > > > > > 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. > > So what do you suggest? Can we change it, or we need to forever live > with divergent definitions in DRM vs elsewhere? > > Gstreamer appears to have a Y410 definition including alpha[1], and > there's the aforementioned Microsoft page too. > > To me it looks like we should change DRM, and if your HW supports > something that's "almost but not quite" Y410 then you need a different > name or only allow alpha-blend-mode "none". > > Thanks, > -Brian > > [1] https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/commit/0ac7d1187b234e86157ad74937c249a3c016807c
Eh. I'm not familiar enough with the gitlab UI. Looks like this didn't merge in GStreamer so far.
> > > > > -Brian > > > > > > [1] https://docs.microsoft.com/en-us/windows/desktop/medfound/10-bit-and-16-bit-yuv-video-formats#444-formats > > > >
| |