Re: [igt-dev] [PATCH v6 03/13] fb: Create common function to convert frame formats

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Paul Kocialkowski <paul.kocialkowski@bootlin.com>,
	eben@raspberrypi.org, igt-dev@lists.freedesktop.org,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Subject: Re: [igt-dev] [PATCH v6 03/13] fb: Create common function to convert frame formats
Date: Thu, 6 Sep 2018 16:30:40 +0300	[thread overview]
Message-ID: <20180906133040.GA5565@intel.com> (raw)
In-Reply-To: <20180906124917.bn7siyjeaqafmkpg@flea>

On Thu, Sep 06, 2018 at 02:49:17PM +0200, Maxime Ripard wrote:
> On Thu, Sep 06, 2018 at 03:11:16PM +0300, Ville Syrjälä wrote:
> > On Thu, Sep 06, 2018 at 01:41:29PM +0200, Maxime Ripard wrote:
> > > On Fri, Aug 31, 2018 at 04:41:10PM +0300, Ville Syrjälä wrote:
> > > > On Fri, Aug 31, 2018 at 03:12:12PM +0200, Maxime Ripard wrote:
> > > > > The current code to convert between two buffer formats is quite tied to the
> > > > > cairo surface infrastructure. Since we'll want to reuse it, make that
> > > > > function more generic by introducing a common structure that passes all the
> > > > > arguments and a common function that will call the right functions we
> > > > > needed.
> > > > > 
> > > > > Reviewed-by: Eric Anholt <eric@anholt.net>
> > > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > > > > ---
> > > > >  lib/igt_fb.c | 243 ++++++++++++++++++++++++++++++++--------------------
> > > > >  1 file changed, 153 insertions(+), 90 deletions(-)
> > > > > 
> > > > > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > > > > index 4061fedec0c1..1914233786a5 100644
> > > > > --- a/lib/igt_fb.c
> > > > > +++ b/lib/igt_fb.c
> > > > > @@ -1384,6 +1384,24 @@ struct fb_convert_blit_upload {
> > > > >  	struct fb_blit_linear linear;
> > > > >  };
> > > > >  
> > > > > +struct fb_convert_buf {
> > > > > +	void			*ptr;
> > > > > +	unsigned int		stride;
> > > > > +	unsigned int		size;
> > > > > +	uint32_t		fmt;
> > > > > +	enum igt_color_encoding	color_encoding;
> > > > > +	enum igt_color_range	color_range;
> > > > > +	uint32_t		offsets[4];
> > > > > +};
> > > > 
> > > > I wonder if we can just use igt_fb for that instead of duplicating most
> > > > of it?
> > > 
> > > After looking into it again, now I remember why I didn't do what you
> > > suggested. In the case where you are in the YUYV path, with a "shadow"
> > > buffer being used to perform the cairo operations on top of an YUV
> > > buffer, you end up with conversions in create_cairo_surface__convert
> > > and destroy_cairo_surface__convert, getting a fb_convert_blit_upload
> > > structure as an argument.
> > > 
> > > That structure however is used to convert one buffer to the shadow
> > > buffer, or the other way around at destroy time. However, a single
> > > igt_fb is allocated for that structure, which makes it impossible to
> > > assign the second fb in that particular case. All the informations
> > > needed to perform the conversion are stored in multiple structures
> > > (fb_convert_blit_upload.linear, fb_convert_blit_upload.blit, and the
> > > actual igt_fb instance) that we need to aggregate to find the correct
> > > conversion parameters.
> > 
> > The patch series I mentioned before [1] changes fb_blit_linear to inherit
> > from igt_fb. So presumably that should help?
> > 
> > [1] https://patchwork.freedesktop.org/series/46876/
> 
> Yeah, except that the shadow buffer would still not have a backing
> igt_fb,

Ah, the rgb24 thing? Hmm. I guess we could make that into an igt_fb as
well.

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