[Intel-gfx] [PATCH] drm/fourcc: Fix conflicting Y41x definitions
Maarten Lankhorst
maarten.lankhorst at linux.intel.com
Wed Mar 20 10:28:43 UTC 2019
Op 19-03-2019 om 17:18 schreef Ville Syrjälä:
> 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 at 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 at 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 at 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 at linux.intel.com>
>>>> Cc: Brian Starkey <Brian.Starkey at arm.com>
>>>> Cc: Swati Sharma <swati2.sharma at intel.com>
>>>> Cc: Ayan Kumar Halder <ayan.halder at arm.com>
>>>> Cc: malidp at foss.arm.com
>>>> Cc: Daniel Vetter <daniel at ffwll.ch>
>>>> Cc: Maxime Ripard <maxime.ripard at bootlin.com>
>>>> Cc: Sean Paul <sean at poorly.run>
>>>> Cc: Dave Airlie <airlied at linux.ie>
>>>> Cc: Liviu Dudau <Liviu.Dudau at 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.
>
Fun.
Is the original name XYUV12_16161616 ok with you? would just like to move it forward.
More information about the Intel-gfx
mailing list