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

[Dhinakaran Pandiyan]

On Fri, 2019-02-22 at 21:34 +0200, Ville Syrjälä wrote:
> 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.

Sounds good, thanks for the reviews.

I'll wait for the remaining patches(1, 2 and 7) are reviewed and push
the series.

-DK