Re: [igt-dev] [HAX 3/5] lib/igt_fb: Use rendercopy for rendering into compressed buffers

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com>
Cc: "igt-dev@lists.freedesktop.org" <igt-dev@lists.freedesktop.org>
Subject: Re: [igt-dev] [HAX 3/5] lib/igt_fb: Use rendercopy for rendering into compressed buffers
Date: Fri, 22 Feb 2019 21:34:54 +0200	[thread overview]
Message-ID: <20190222193454.GY20097@intel.com> (raw)
In-Reply-To: <721e94de08d17eb7408046eb83b7de498bedb4ac.camel@intel.com>

On Fri, Feb 22, 2019 at 07:12:04PM +0000, Pandiyan, Dhinakaran wrote:
> On Fri, 2019-02-22 at 20:54 +0200, Ville Syrjälä wrote:
> > On Fri, Feb 22, 2019 at 10:37:40AM -0800, Dhinakaran Pandiyan wrote:
> > > On Fri, 2019-02-22 at 19:26 +0200, Ville Syrjälä wrote:
> > > > On Thu, Feb 21, 2019 at 03:20:35PM -0800, Dhinakaran Pandiyan
> > > > wrote:
> > > > > On Thu, 2019-02-21 at 16:06 +0200, Ville Syrjälä wrote:
> > > > > > On Wed, Feb 20, 2019 at 06:41:21PM -0800, Dhinakaran Pandiyan
> > > > > > wrote:
> > > > > > > @@ -2397,8 +2523,10 @@ cairo_surface_t
> > > > > > > *igt_get_cairo_surface(int
> > > > > > > fd, struct igt_fb *fb)
> > > > > > >                ((f->cairo_id == CAIRO_FORMAT_INVALID) &&
> > > > > > >                 (f->pixman_id != PIXMAN_invalid)))
> > > > > > >                    create_cairo_surface__convert(fd, fb);
> > > > > 
> > > > > All BGR formats will take this path and it somehow results in
> > > > > Yf-
> > > > > CCS
> > > > > tests specifically failing.
> > > > 
> > > > Hmm. Should be a matter of 
> > > > 
> > > >  if (...
> > > > +    is_ccs) {
> > > >         setup_linear_mapping();
> > > >  } else {
> > > >         ...
> > > > 
> > > > no? Or was that already in there?
> > > > 
> > > > It is slightly suboptimal since in theory we could just use 
> > > > rendercopy to do the full conversion for us, skipping the
> > > > expensive
> > > > software step. IIRC I briefly mentioned this option when Maarten
> > > > was adding the !32bpp support to rendercopy. Anyways, we could
> > > > consider doing that in the future if we are willing to expand
> > > > our use of rendercopy. The counter argument is that it's one
> > > > more extra thing that could fail.
> > > 
> > > Didn't quite get why rendercopy would have a higher chance of
> > > failing.
> > > Personally, the problem I have is I don't fully understand the
> > > rendercopy code yet :) And, I am not sure how easy it is to extend
> > > support for new platforms from a display IGT development point of
> > > view.
> > 
> > It all depends how much the hw people feel the need to change.
> > Hopefully
> > not too much, and that someone has the energy to find out which new
> > magic bits are needed to make it work.
> > 
> > Also if the GT side is having major issues we can't rely on
> > rendercopy
> > obviously. Just one more thing to worry about, at least during early
> > testing/development.
> > 
> > > I guess the library is not backward compatible either.
> > 
> > In what sense?
> Are things like copying from one legacy tiling format to another
> expected to work on new platforms with the same code? Based on your
> reply above, I understand that it depends on how much the HW has
> changed.

Yeah. If they feel like removing old tiling formats rendercopy
can't help us.

> 
> > 
> 
> And do you plan to review 
> https://patchwork.freedesktop.org/patch/287379/? Asking because you
> wrote most of that patch.

It has my sob, so should be good to go.

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev