On 23.02.2018 05:06, Srinivas, Vidya wrote:
-----Original Message-----
From: Juha-Pekka Heikkila [mailto:juhapekka.heikkila@xxxxxxxxx]
Sent: Thursday, February 22, 2018 7:06 PM
To: Srinivas, Vidya <vidya.srinivas@xxxxxxxxx>; intel-
gfx@xxxxxxxxxxxxxxxxxxxxx
Subject: Re: [PATCH 13/16] drm/i915: Add NV12 as supported
format for primary plane
On 22.02.2018 04:39, Srinivas, Vidya wrote:
-----Original Message-----
From: Juha-Pekka Heikkila [mailto:juhapekka.heikkila@xxxxxxxxx]
Sent: Wednesday, February 21, 2018 7:52 PM
To: Srinivas, Vidya <vidya.srinivas@xxxxxxxxx>; intel-
gfx@xxxxxxxxxxxxxxxxxxxxx
Subject: Re: [PATCH 13/16] drm/i915: Add NV12 as
supported format for primary plane
On 21.02.2018 12:20, Vidya Srinivas wrote:
From: Chandra Konduru <chandra.konduru@xxxxxxxxx>
This patch adds NV12 to list of supported formats for primary plane
v2: Rebased (Chandra Konduru)
v3: Rebased (me)
v4: Review comments by Ville addressed Removed the
skl_primary_formats_with_nv12 and added NV12 case in existing
skl_primary_formats
v5: Rebased (me)
v6: Missed the Tested-by/Reviewed-by in the previous series Adding
the same to commit message in this version.
v7: Review comments by Ville addressed
Restricting the NV12 for BXT and on PIPE A and B Rebased (me)
v8: Rebased (me)
Modified restricting the NV12 support for both BXT and KBL.
v9: Rebased (me)
v10: Addressed review comments from Maarten.
Adding NV12 inside skl_primary_formats itself.
v11: Adding Reviewed By tag from Shashank Sharma
Tested-by: Clinton Taylor <clinton.a.taylor@xxxxxxxxx>
Reviewed-by: Clinton Taylor <clinton.a.taylor@xxxxxxxxx>
Reviewed-by: Shashank Sharma <shashank.sharma@xxxxxxxxx>
Signed-off-by: Chandra Konduru <chandra.konduru@xxxxxxxxx>
Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@xxxxxxxxx>
Signed-off-by: Vidya Srinivas <vidya.srinivas@xxxxxxxxx>
---
drivers/gpu/drm/i915/intel_display.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 142dfe0..1870366 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -86,6 +86,7 @@ static const uint32_t skl_primary_formats[] = {
DRM_FORMAT_YVYU,
DRM_FORMAT_UYVY,
DRM_FORMAT_VYUY,
+ DRM_FORMAT_NV12,
};
static const uint64_t skl_format_modifiers_noccs[] = { @@
-13282,6
+13283,10 @@ intel_primary_plane_create(struct drm_i915_private
*dev_priv, enum pipe pipe)
intel_primary_formats = skl_primary_formats;
num_formats = ARRAY_SIZE(skl_primary_formats);
+ if ((INTEL_GEN(dev_priv) == 9 && pipe == PIPE_C) &&
+ !IS_GEMINILAKE(dev_priv))
+ num_formats -= 1;
This doesn't look future proof solution. This creates invisible
dependency where it is required NV12 is last item in list of formats.
Initially we had a different array for this. But as a part of one of
the review comments, I made this change.
I did see Maarten's comment on your older patch. In my opinion having two
lists would be more clear. Regardless of opinions on which is better you
maybe anyway want to reconsider this piece of code as this expose
NV12 also for Skylake platform.
Maarten, could you please add your inputs as well?
I will change the patch according to what we decide.
Thank you.
If other's see it better to keep it in one list I'm ok for that. To me
it just feel like accident waiting to happen if this list is changed at
later time for different reasons.
My other comment about Skylake platform, you shouldn't enable NV12 for
Skylake platform where its documented to have problems (I mean not all
users of skl_* list but rule out probably something like
IS_SKYLAKE(..)). For this reason this patch shouldn't be used as is.
This same story applies for patch #14 of this series, NV12 shouldn't be
enabled on Skylake platform.
+
if (skl_plane_has_ccs(dev_priv, pipe, PLANE_PRIMARY))
modifiers = skl_format_modifiers_ccs;
else
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
