On Fri, Aug 14, 2020 at 05:40:34PM +0200, Karol Herbst wrote:
> On Fri, Aug 14, 2020 at 5:35 PM Thierry Reding <thierry.reding@gmail.com> wrote:
> >
> > On Fri, Aug 14, 2020 at 04:44:43PM +0200, Karol Herbst wrote:
> > > On Fri, Aug 14, 2020 at 4:05 PM Thierry Reding <thierry.reding@gmail.com> wrote:
> > > >
> > > > On Thu, Aug 13, 2020 at 10:45:34AM -0700, James Jones wrote:
> > > > > I'll defer to Thierry, but I think that may be by design.  Tegra format
> > > > > modifiers were added to get things like this working in the first place,
> > > > > right?  It's not a regression, is it?
> > > >
> > > > I recall that things used to work with or without modifiers at some
> > > > point. That was basically the point of the "pitch-linear by default"
> > > > patch, see:
> > > >
> > > >     https://gitlab.freedesktop.org/mesa/mesa/-/commit/9603d81df05105857b676f20dff964ef3ab0ecff
> > > >
> > > > If we don't force pitch-linear for cases where we don't have modifiers,
> > > > there's no way we can properly display these as framebuffers because we
> > > > lack the modifier information at the kernel level.
> > > >
> > >
> > > I suspect that's related to
> > > https://gitlab.freedesktop.org/mesa/mesa/commit/c56fe4118a2dd991ff1b2a532c0f234eddd435e9
> >
> > But we're checking the the PIPE_USAGE_SCANOUT flag specifically for that
> > type of case, so this should work. On the other hand, it does sound like
> > that logic might actually work, but for some reason Nouveau then ends up
> > allocating a linear render buffer and a tiled depth buffer, and that's
> > the combination that doesn't work. Did I understand that correctly?
> >
> > So it sounds like there's a few options:
> >
> >   a) Make Nouveau render without a depth buffer in these cases (not sure
> >      if that's even possible).
> >
> >   b) Allocate a linear buffer *in addition* to the normal buffer that
> >      Nouveau allocates (with whatever it selects as the default layout)
> >      and then blit from the tiled buffer to the linear buffer everytime
> >      we need to. I think that'd be basically reproducing the code that
> >      is controlled by the DRI_PRIME environment variable.
> >
> 
> yeah, I think Ilia suggested something like that a long time ago as well.
> 
> >   c) Accept that modifiers are the way to go and rely on them for proper
> >      functionality. This is obviously a really bad solution because not
> >      everyone has transitioned to framebuffer modifiers yet. On the
> >      other hand, this is precisely the kind of use-case that framebuffer
> >      modifiers were meant to address, so it isn't all that far-fetched
> >      to make them a requirement.
> >
> 
> the problem with that is, that by default we don't have modifier aware
> desktops. gnome-shell by default and no Xorg release.
> 
> > None of those are really great options... does anyone have any other
> > ideas?
> >
> 
> What I am wondering about is why with my patch it simply works in X,
> but that could be a mere coincidence.

So I was testing your revert with Weston and that "works", but only as
in "it doesn't crash". As expected, since there's now a mismatch between
what layout Nouveau assumes and the pitch-linear buffer that modesetting
assumes it got, it'll now be completely scrambled. That said, even
without the revert I can't seem to be able to make Weston work without
modifiers support.

I'll have to investigate some more why that's not working. Perhaps it
doesn't pass the correct usage flags?

Thierry