public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <maxime.ripard@bootlin.com>,
	intel-gfx@lists.freedesktop.org, Dave Airlie <airlied@linux.ie>,
	malidp@foss.arm.com, dri-devel@lists.freedesktop.org,
	Ayan Kumar Halder <ayan.halder@arm.com>
Subject: Re: [PATCH] drm/fourcc: Fix conflicting Y41x definitions
Date: Tue, 19 Mar 2019 18:18:38 +0200	[thread overview]
Message-ID: <20190319161838.GE3888@intel.com> (raw)
In-Reply-To: <92b61ab7-7127-0523-0d64-54ef7ca782e7@linux.intel.com>

On Tue, Mar 19, 2019 at 05:06:36PM +0100, Maarten Lankhorst wrote:
> Op 19-03-2019 om 14:02 schreef Ville Syrjälä:
> > On Tue, Mar 19, 2019 at 01:17:02PM +0100, Maarten Lankhorst wrote:
> >> There has unfortunately been a conflict with the following 3 commits:
> >>
> >> commit e9961ab95af81b8d29054361cd5f0c575102cf87
> >> Author: Ayan Kumar Halder <ayan.halder@arm.com>
> >> Date:   Fri Nov 9 17:21:12 2018 +0000
> >>     drm: Added a new format DRM_FORMAT_XVYU2101010
> >>
> >> commit 7ba0fee247ee7a36b3bfbed68f6988d980aa3aa3
> >> Author: Brian Starkey <brian.starkey@arm.com>
> >> Date:   Fri Oct 5 10:27:00 2018 +0100
> >>
> >>     drm/fourcc: Add AFBC yuv fourccs for Mali
> >>
> >> and
> >>
> >> commit 50bf5d7d595fd0705ef3785f80e679b6da501e5b
> >> Author: Swati Sharma <swati2.sharma@intel.com>
> >> Date:   Mon Mar 4 17:26:33 2019 +0530
> >>
> >>     drm: Add Y2xx and Y4xx (xx:10/12/16) format definitions and fourcc
> >>
> >> Unfortunately gcc didn't warn about the redefinitions, because the
> >>
> >> Fix this by using new XYVU for i915, without alpha, and making the
> >> Y41x definitions match msdn, with alpha.
> > The naming of all these is rather unfortunate because now the alpha vs.
> > non-alpha formats have totally different looking names :( Fourccs are
> > stupid!
> >
> >> Fortunately we caught it early, and the conflict hasn't even landed in
> >> drm-next yet.
> >>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> Cc: Brian Starkey <Brian.Starkey@arm.com>
> >> Cc: Swati Sharma <swati2.sharma@intel.com>
> >> Cc: Ayan Kumar Halder <ayan.halder@arm.com>
> >> Cc: malidp@foss.arm.com
> >> Cc: Daniel Vetter <daniel@ffwll.ch>
> >> Cc: Maxime Ripard <maxime.ripard@bootlin.com>
> >> Cc: Sean Paul <sean@poorly.run>
> >> Cc: Dave Airlie <airlied@linux.ie>
> >> Cc: Liviu Dudau <Liviu.Dudau@arm.com>
> >> ---
> >>  drivers/gpu/drm/drm_fourcc.c         | 12 +++++------
> >>  drivers/gpu/drm/i915/intel_display.c | 18 ++++++++---------
> >>  drivers/gpu/drm/i915/intel_sprite.c  | 30 ++++++++++++++--------------
> >>  include/uapi/drm/drm_fourcc.h        | 21 +++++++++----------
> >>  4 files changed, 41 insertions(+), 40 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> >> index b914b16db9b2..6ea55fb4526d 100644
> >> --- a/drivers/gpu/drm/drm_fourcc.c
> >> +++ b/drivers/gpu/drm/drm_fourcc.c
> >> @@ -229,17 +229,17 @@ const struct drm_format_info *__drm_format_info(u32 format)
> >>  		{ .format = DRM_FORMAT_UYVY,		.depth = 0,  .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
> >>  		{ .format = DRM_FORMAT_VYUY,		.depth = 0,  .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
> >>  		{ .format = DRM_FORMAT_XYUV8888,	.depth = 0,  .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .is_yuv = true },
> >> -		{ .format = DRM_FORMAT_Y210,		.depth = 0,  .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
> >>  		{ .format = DRM_FORMAT_VUY888,          .depth = 0,  .num_planes = 1, .cpp = { 3, 0, 0 }, .hsub = 1, .vsub = 1, .is_yuv = true },
> >> -		{ .format = DRM_FORMAT_Y410,            .depth = 0,  .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true, .is_yuv = true },
> >>  		{ .format = DRM_FORMAT_AYUV,		.depth = 0,  .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true, .is_yuv = true },
> >> -		{ .format = DRM_FORMAT_XVYU2101010,	.depth = 0,  .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .is_yuv = true },
> >>  		{ .format = DRM_FORMAT_Y210,            .depth = 0,  .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
> >>  		{ .format = DRM_FORMAT_Y212,            .depth = 0,  .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
> >>  		{ .format = DRM_FORMAT_Y216,            .depth = 0,  .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
> >> -		{ .format = DRM_FORMAT_Y410,            .depth = 0,  .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .is_yuv = true },
> >> -		{ .format = DRM_FORMAT_Y412,            .depth = 0,  .num_planes = 1, .cpp = { 8, 0, 0 }, .hsub = 1, .vsub = 1, .is_yuv = true },
> >> -		{ .format = DRM_FORMAT_Y416,            .depth = 0,  .num_planes = 1, .cpp = { 8, 0, 0 }, .hsub = 1, .vsub = 1, .is_yuv = true },
> >> +		{ .format = DRM_FORMAT_Y410,            .depth = 0,  .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true, .is_yuv = true },
> >> +		{ .format = DRM_FORMAT_Y412,            .depth = 0,  .num_planes = 1, .cpp = { 8, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true, .is_yuv = true },
> >> +		{ .format = DRM_FORMAT_Y416,            .depth = 0,  .num_planes = 1, .cpp = { 8, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true, .is_yuv = true },
> >> +		{ .format = DRM_FORMAT_XVYU2101010,	.depth = 0,  .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .is_yuv = true },
> >> +		{ .format = DRM_FORMAT_XVYU12_16161616,	.depth = 0,  .num_planes = 1, .cpp = { 8, 0, 0 }, .hsub = 1, .vsub = 1, .is_yuv = true },
> > Damn these 10/12 in 16 formats are annoying. 
> >
> > Maybe it would look nicer to put the 12 at the end?
> 
> Either way sucks..
> 
> Could we keep it where it is so I don't have to respin? :)
> 
> >> +		{ .format = DRM_FORMAT_XVYU16161616,	.depth = 0,  .num_planes = 1, .cpp = { 8, 0, 0 }, .hsub = 1, .vsub = 1, .is_yuv = true },
> >>  		{ .format = DRM_FORMAT_Y0L0,		.depth = 0,  .num_planes = 1,
> >>  		  .char_per_block = { 8, 0, 0 }, .block_w = { 2, 0, 0 }, .block_h = { 2, 0, 0 },
> >>  		  .hsub = 2, .vsub = 2, .has_alpha = true, .is_yuv = true },
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> index a5ad18c3bf44..94496488641c 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -2690,11 +2690,11 @@ int skl_format_to_fourcc(int format, bool rgb_order, bool alpha)
> >>  	case PLANE_CTL_FORMAT_Y216:
> >>  		return DRM_FORMAT_Y216;
> >>  	case PLANE_CTL_FORMAT_Y410:
> >> -		return DRM_FORMAT_Y410;
> >> +		return DRM_FORMAT_XVYU2101010;
> >>  	case PLANE_CTL_FORMAT_Y412:
> >> -		return DRM_FORMAT_Y412;
> >> +		return DRM_FORMAT_XVYU12_16161616;
> >>  	case PLANE_CTL_FORMAT_Y416:
> >> -		return DRM_FORMAT_Y416;
> >> +		return DRM_FORMAT_XVYU16161616;
> >>  	default:
> >>  	case PLANE_CTL_FORMAT_XRGB_8888:
> >>  		if (rgb_order) {
> >> @@ -3648,11 +3648,11 @@ static u32 skl_plane_ctl_format(u32 pixel_format)
> >>  		return PLANE_CTL_FORMAT_Y212;
> >>  	case DRM_FORMAT_Y216:
> >>  		return PLANE_CTL_FORMAT_Y216;
> >> -	case DRM_FORMAT_Y410:
> >> +	case DRM_FORMAT_XVYU2101010:
> >>  		return PLANE_CTL_FORMAT_Y410;
> >> -	case DRM_FORMAT_Y412:
> >> +	case DRM_FORMAT_XVYU12_16161616:
> >>  		return PLANE_CTL_FORMAT_Y412;
> >> -	case DRM_FORMAT_Y416:
> >> +	case DRM_FORMAT_XVYU16161616:
> >>  		return PLANE_CTL_FORMAT_Y416;
> >>  	default:
> >>  		MISSING_CASE(pixel_format);
> >> @@ -5216,9 +5216,9 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
> >>  	case DRM_FORMAT_Y210:
> >>  	case DRM_FORMAT_Y212:
> >>  	case DRM_FORMAT_Y216:
> >> -	case DRM_FORMAT_Y410:
> >> -	case DRM_FORMAT_Y412:
> >> -	case DRM_FORMAT_Y416:
> >> +	case DRM_FORMAT_XVYU2101010:
> >> +	case DRM_FORMAT_XVYU12_16161616:
> >> +	case DRM_FORMAT_XVYU16161616:
> >>  		break;
> >>  	default:
> >>  		DRM_DEBUG_KMS("[PLANE:%d:%s] FB:%d unsupported scaling format 0x%x\n",
> >> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> >> index 9892c88ede1d..53174d579574 100644
> >> --- a/drivers/gpu/drm/i915/intel_sprite.c
> >> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> >> @@ -1821,9 +1821,9 @@ static const uint32_t icl_plane_formats[] = {
> >>  	DRM_FORMAT_Y210,
> >>  	DRM_FORMAT_Y212,
> >>  	DRM_FORMAT_Y216,
> >> -	DRM_FORMAT_Y410,
> >> -	DRM_FORMAT_Y412,
> >> -	DRM_FORMAT_Y416,
> >> +	DRM_FORMAT_XVYU2101010,
> >> +	DRM_FORMAT_XVYU12_16161616,
> >> +	DRM_FORMAT_XVYU16161616,
> > The spec says only HDR planes support 64bpp formats. So either the spec
> > is wrong or we have way too many formats on the SDR plane format lists.
> 
> It seems kms_available_modes_crc passes with those formats on icl-y, so I'm assuming it works?

JP said a bunch of yuv things fail again. So either my fixes didn't
really fix things or stuff regressed again.

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

  reply	other threads:[~2019-03-19 16:18 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-19 12:17 [PATCH] drm/fourcc: Fix conflicting Y41x definitions Maarten Lankhorst
2019-03-19 12:40 ` ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2019-03-19 13:02 ` [PATCH] " Ville Syrjälä
2019-03-19 16:06   ` Maarten Lankhorst
2019-03-19 16:18     ` Ville Syrjälä [this message]
2019-03-20 10:28       ` Maarten Lankhorst
2019-03-19 13:09 ` ✓ Fi.CI.BAT: success for " Patchwork
2019-03-19 13:58 ` [PATCH] " Ayan Halder
2019-03-19 20:12 ` ✗ Fi.CI.IGT: failure for " Patchwork
2019-03-20 15:48 ` [PATCH] " Adam Jackson
2019-03-20 16:00   ` Maarten Lankhorst
     [not found] ` <20190320182207.GL114153@art_vandelay>
2019-03-21  9:14   ` Maarten Lankhorst

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190319161838.GE3888@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=airlied@linux.ie \
    --cc=ayan.halder@arm.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=malidp@foss.arm.com \
    --cc=maxime.ripard@bootlin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox