Re: [PATCH] drm: rcar-du: Fix pitch handling for fully planar YUV formats

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Sep 08, 2020 at 05:05:48PM +0100, Kieran Bingham wrote:
> Hi Laurent,
> 
> On 08/09/2020 16:52, Laurent Pinchart wrote:
> > Hi Kieran,
> > 
> > On Tue, Sep 08, 2020 at 04:42:58PM +0100, Kieran Bingham wrote:
> >> On 06/08/2020 03:26, Laurent Pinchart wrote:
> >>> When creating a frame buffer, the driver verifies that the pitches for
> >>> the chroma planes match the luma plane. This is done incorrectly for
> >>> fully planar YUV formats, without taking horizontal subsampling into
> >>> account. Fix it.
> >>>
> >>> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> >>> ---
<snip>
> >>>  	}, {
> >>>  		.fourcc = DRM_FORMAT_YVU444,
> >>>  		.v4l2 = V4L2_PIX_FMT_YVU444M,
> >>>  		.bpp = 24,
> >>>  		.planes = 3,
> >>> +		.hsub = 1,
> >>>  	},
> >>>  };
> >>>  
> >>
> >> I wonder when we can have a global/generic set of format tables so that
> >> all of this isn't duplicated on a per-driver basis.
> > 
> > Note that this table also contains register values, so at least that
> > part will need to be kept. For the rest, do you mean a 4CC library that
> 
> Yes, the driver specific mappings of course need to be driver specific.
> 
> 
> > would be shared between DRM/KMS and V4L2 ? That's a great idea. Too bad
> > it has been shot down when patches were submitted :-S
> 
> 
>  /o\ ... It just seems like so much data replication that must be used
> by many drivers.
> 
> Even without mapping the DRM/V4L2 fourccs - even a common table in each
> subsystem would be beneficial wouldn't it?
> 
> I mean - RCar-DU isn't the only device that needs to know how many
> planes DRM_FORMAT_YUV422 has, or what horizontal subsampling it uses?
> 
> Anyway, that's not an issue with this patch, it just seems glaring to me
> that these entries are common across all hardware that use them ...
> 
> (the bpp/planes/subsampling of course, not the hardware specific registers).

See drm_format_info() & co.

-- 
Ville Syrjälä
Intel