diff mbox series

[7/9] drm/i915: Reject ckey+fp16 on skl+

Message ID 20191008161441.12721-7-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [1/9] drm/i915: Expose 10:10:10 XRGB formats on SNB-BDW sprites | expand

Commit Message

Ville Syrjälä Oct. 8, 2019, 4:14 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

According to the spec color keying is not supported with
fp16 pixel formats on skl+. Reject that combo.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_sprite.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Comments

Shankar, Uma Oct. 29, 2019, 1:07 p.m. UTC | #1 
>-----Original Message-----
>From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville Syrjala
>Sent: Tuesday, October 8, 2019 9:45 PM
>To: intel-gfx@lists.freedesktop.org
>Subject: [Intel-gfx] [PATCH 7/9] drm/i915: Reject ckey+fp16 on skl+
>
>From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
>According to the spec color keying is not supported with
>fp16 pixel formats on skl+. Reject that combo.
>
>Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>---
> drivers/gpu/drm/i915/display/intel_sprite.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c
>b/drivers/gpu/drm/i915/display/intel_sprite.c
>index cc9e5c9668b1..d6cd46e3f738 100644
>--- a/drivers/gpu/drm/i915/display/intel_sprite.c
>+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
>@@ -1689,6 +1689,19 @@ vlv_sprite_check(struct intel_crtc_state *crtc_state,
> 	return 0;
> }
>
>+static bool format_is_fp16(u32 format)
>+{
>+	switch (format) {
>+	case DRM_FORMAT_XRGB16161616F:
>+	case DRM_FORMAT_XBGR16161616F:
>+	case DRM_FORMAT_ARGB16161616F:
>+	case DRM_FORMAT_ABGR16161616F:
>+		return true;
>+	default:
>+		return false;
>+	}
>+}
>+
> static int skl_plane_check_fb(const struct intel_crtc_state *crtc_state,
> 			      const struct intel_plane_state *plane_state)  { @@ -
>1760,6 +1773,11 @@ static int skl_plane_check_fb(const struct intel_crtc_state
>*crtc_state,
> 		return -EINVAL;
> 	}
>
>+	if (plane_state->ckey.flags && format_is_fp16(fb->format->format)) {
>+		DRM_DEBUG_KMS("Color keying not supported with fp16
>formats\n");

It seems even "Indexed 8 bit formats" also don't support Color Keying. May be you can extend it to
even C8.

>+		return -EINVAL;
>+	}
>+
> 	return 0;
> }
>
>--
>2.21.0
>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Shankar, Uma Oct. 29, 2019, 1:35 p.m. UTC | #2 
>-----Original Message-----
>From: Shankar, Uma
>Sent: Tuesday, October 29, 2019 6:38 PM
>To: Ville Syrjala <ville.syrjala@linux.intel.com>; intel-gfx@lists.freedesktop.org
>Subject: RE: [Intel-gfx] [PATCH 7/9] drm/i915: Reject ckey+fp16 on skl+
>
>
>
>>-----Original Message-----
>>From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of
>>Ville Syrjala
>>Sent: Tuesday, October 8, 2019 9:45 PM
>>To: intel-gfx@lists.freedesktop.org
>>Subject: [Intel-gfx] [PATCH 7/9] drm/i915: Reject ckey+fp16 on skl+
>>
>>From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>>According to the spec color keying is not supported with
>>fp16 pixel formats on skl+. Reject that combo.
>>
>>Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>---
>> drivers/gpu/drm/i915/display/intel_sprite.c | 18 ++++++++++++++++++
>> 1 file changed, 18 insertions(+)
>>
>>diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c
>>b/drivers/gpu/drm/i915/display/intel_sprite.c
>>index cc9e5c9668b1..d6cd46e3f738 100644
>>--- a/drivers/gpu/drm/i915/display/intel_sprite.c
>>+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
>>@@ -1689,6 +1689,19 @@ vlv_sprite_check(struct intel_crtc_state *crtc_state,
>> 	return 0;
>> }
>>
>>+static bool format_is_fp16(u32 format) {
>>+	switch (format) {
>>+	case DRM_FORMAT_XRGB16161616F:
>>+	case DRM_FORMAT_XBGR16161616F:
>>+	case DRM_FORMAT_ARGB16161616F:
>>+	case DRM_FORMAT_ABGR16161616F:
>>+		return true;
>>+	default:
>>+		return false;
>>+	}
>>+}
>>+
>> static int skl_plane_check_fb(const struct intel_crtc_state *crtc_state,
>> 			      const struct intel_plane_state *plane_state)  { @@ -
>>1760,6 +1773,11 @@ static int skl_plane_check_fb(const struct
>>intel_crtc_state *crtc_state,
>> 		return -EINVAL;
>> 	}
>>
>>+	if (plane_state->ckey.flags && format_is_fp16(fb->format->format)) {
>>+		DRM_DEBUG_KMS("Color keying not supported with fp16
>>formats\n");
>
>It seems even "Indexed 8 bit formats" also don't support Color Keying. May be you
>can extend it to even C8.

wrt C8, at the bit definition of color keying on PLANE_CTL the description says
"Plane color keying is not compatible with the Indexed 8-bit pixel format.",
but on capability it do list C8. So not sure what is correct. 

>
>>+		return -EINVAL;
>>+	}
>>+
>> 	return 0;
>> }
>>
>>--
>>2.21.0
>>
>>_______________________________________________
>>Intel-gfx mailing list
>>Intel-gfx@lists.freedesktop.org
>>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Ville Syrjälä Oct. 29, 2019, 3:22 p.m. UTC | #3 
On Tue, Oct 29, 2019 at 01:35:57PM +0000, Shankar, Uma wrote:
> 
> 
> >-----Original Message-----
> >From: Shankar, Uma
> >Sent: Tuesday, October 29, 2019 6:38 PM
> >To: Ville Syrjala <ville.syrjala@linux.intel.com>; intel-gfx@lists.freedesktop.org
> >Subject: RE: [Intel-gfx] [PATCH 7/9] drm/i915: Reject ckey+fp16 on skl+
> >
> >
> >
> >>-----Original Message-----
> >>From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of
> >>Ville Syrjala
> >>Sent: Tuesday, October 8, 2019 9:45 PM
> >>To: intel-gfx@lists.freedesktop.org
> >>Subject: [Intel-gfx] [PATCH 7/9] drm/i915: Reject ckey+fp16 on skl+
> >>
> >>From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>
> >>According to the spec color keying is not supported with
> >>fp16 pixel formats on skl+. Reject that combo.
> >>
> >>Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>---
> >> drivers/gpu/drm/i915/display/intel_sprite.c | 18 ++++++++++++++++++
> >> 1 file changed, 18 insertions(+)
> >>
> >>diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c
> >>b/drivers/gpu/drm/i915/display/intel_sprite.c
> >>index cc9e5c9668b1..d6cd46e3f738 100644
> >>--- a/drivers/gpu/drm/i915/display/intel_sprite.c
> >>+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
> >>@@ -1689,6 +1689,19 @@ vlv_sprite_check(struct intel_crtc_state *crtc_state,
> >> 	return 0;
> >> }
> >>
> >>+static bool format_is_fp16(u32 format) {
> >>+	switch (format) {
> >>+	case DRM_FORMAT_XRGB16161616F:
> >>+	case DRM_FORMAT_XBGR16161616F:
> >>+	case DRM_FORMAT_ARGB16161616F:
> >>+	case DRM_FORMAT_ABGR16161616F:
> >>+		return true;
> >>+	default:
> >>+		return false;
> >>+	}
> >>+}
> >>+
> >> static int skl_plane_check_fb(const struct intel_crtc_state *crtc_state,
> >> 			      const struct intel_plane_state *plane_state)  { @@ -
> >>1760,6 +1773,11 @@ static int skl_plane_check_fb(const struct
> >>intel_crtc_state *crtc_state,
> >> 		return -EINVAL;
> >> 	}
> >>
> >>+	if (plane_state->ckey.flags && format_is_fp16(fb->format->format)) {
> >>+		DRM_DEBUG_KMS("Color keying not supported with fp16
> >>formats\n");
> >
> >It seems even "Indexed 8 bit formats" also don't support Color Keying. May be you
> >can extend it to even C8.
> 
> wrt C8, at the bit definition of color keying on PLANE_CTL the description says
> "Plane color keying is not compatible with the Indexed 8-bit pixel format.",
> but on capability it do list C8. So not sure what is correct. 

It works just fine, or at least it did on older platforms.
So unless they broke it recently we should be good.

Regarding fp16 vs. colorkey, not sure what the deal really is.
I should probably test it across the board now that we have
fp16 for all gen4+.
Shankar, Uma Oct. 30, 2019, 3:26 p.m. UTC | #4 
>> >>-----Original Message-----
>> >>From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf
>> >>Of Ville Syrjala
>> >>Sent: Tuesday, October 8, 2019 9:45 PM
>> >>To: intel-gfx@lists.freedesktop.org
>> >>Subject: [Intel-gfx] [PATCH 7/9] drm/i915: Reject ckey+fp16 on skl+
>> >>
>> >>From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >>
>> >>According to the spec color keying is not supported with
>> >>fp16 pixel formats on skl+. Reject that combo.
>> >>
>> >>Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >>---
>> >> drivers/gpu/drm/i915/display/intel_sprite.c | 18 ++++++++++++++++++
>> >> 1 file changed, 18 insertions(+)
>> >>
>> >>diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c
>> >>b/drivers/gpu/drm/i915/display/intel_sprite.c
>> >>index cc9e5c9668b1..d6cd46e3f738 100644
>> >>--- a/drivers/gpu/drm/i915/display/intel_sprite.c
>> >>+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
>> >>@@ -1689,6 +1689,19 @@ vlv_sprite_check(struct intel_crtc_state *crtc_state,
>> >> 	return 0;
>> >> }
>> >>
>> >>+static bool format_is_fp16(u32 format) {
>> >>+	switch (format) {
>> >>+	case DRM_FORMAT_XRGB16161616F:
>> >>+	case DRM_FORMAT_XBGR16161616F:
>> >>+	case DRM_FORMAT_ARGB16161616F:
>> >>+	case DRM_FORMAT_ABGR16161616F:
>> >>+		return true;
>> >>+	default:
>> >>+		return false;
>> >>+	}
>> >>+}
>> >>+
>> >> static int skl_plane_check_fb(const struct intel_crtc_state *crtc_state,
>> >> 			      const struct intel_plane_state *plane_state)  { @@ -
>> >>1760,6 +1773,11 @@ static int skl_plane_check_fb(const struct
>> >>intel_crtc_state *crtc_state,
>> >> 		return -EINVAL;
>> >> 	}
>> >>
>> >>+	if (plane_state->ckey.flags && format_is_fp16(fb->format->format)) {
>> >>+		DRM_DEBUG_KMS("Color keying not supported with fp16
>> >>formats\n");
>> >
>> >It seems even "Indexed 8 bit formats" also don't support Color
>> >Keying. May be you can extend it to even C8.
>>
>> wrt C8, at the bit definition of color keying on PLANE_CTL the
>> description says "Plane color keying is not compatible with the
>> Indexed 8-bit pixel format.", but on capability it do list C8. So not sure what is
>correct.
>
>It works just fine, or at least it did on older platforms.
>So unless they broke it recently we should be good.

Ok, yeah that description was misleading. Will try to get some clarification from
hardware folks as well.

>Regarding fp16 vs. colorkey, not sure what the deal really is.
>I should probably test it across the board now that we have
>fp16 for all gen4+.

That would be great and will confirm the behaviour.

Regards,
Uma Shankar

>--
>Ville Syrjälä
>Intel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
index cc9e5c9668b1..d6cd46e3f738 100644
--- a/drivers/gpu/drm/i915/display/intel_sprite.c
+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
@@ -1689,6 +1689,19 @@  vlv_sprite_check(struct intel_crtc_state *crtc_state,
 	return 0;
 }
 
+static bool format_is_fp16(u32 format)
+{
+	switch (format) {
+	case DRM_FORMAT_XRGB16161616F:
+	case DRM_FORMAT_XBGR16161616F:
+	case DRM_FORMAT_ARGB16161616F:
+	case DRM_FORMAT_ABGR16161616F:
+		return true;
+	default:
+		return false;
+	}
+}
+
 static int skl_plane_check_fb(const struct intel_crtc_state *crtc_state,
 			      const struct intel_plane_state *plane_state)
 {
@@ -1760,6 +1773,11 @@  static int skl_plane_check_fb(const struct intel_crtc_state *crtc_state,
 		return -EINVAL;
 	}
 
+	if (plane_state->ckey.flags && format_is_fp16(fb->format->format)) {
+		DRM_DEBUG_KMS("Color keying not supported with fp16 formats\n");
+		return -EINVAL;
+	}
+
 	return 0;
 }