diff mbox series

[v2,RESEND,2/2] drm/i915/icl: Implement half float formats

Message ID 1549043020-5124-3-git-send-email-kevin.strasser@intel.com (mailing list archive)
State New, archived
Headers show
Series Support 64 bpp half float formats | expand

Commit Message

Kevin Strasser Feb. 1, 2019, 5:43 p.m. UTC
64 bpp half float formats are supported on hdr planes only and are subject
to the following restrictions:
  * 90/270 rotation not supported
  * Yf Tiling not supported
  * Frame Buffer Compression not supported
  * Color Keying not supported

v2:
- Drop handling pixel normalize register
- Don't use icl_is_hdr_plane too early

Signed-off-by: Kevin Strasser <kevin.strasser@intel.com>
Cc: Uma Shankar <uma.shankar@intel.com>
Cc: Shashank Sharma <shashank.sharma@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/i915/intel_display.c | 22 ++++++++++++
 drivers/gpu/drm/i915/intel_sprite.c  | 67 ++++++++++++++++++++++++++++++++----
 2 files changed, 83 insertions(+), 6 deletions(-)

Comments

Ville Syrjälä Feb. 1, 2019, 8:40 p.m. UTC | #1
On Fri, Feb 01, 2019 at 09:43:40AM -0800, Kevin Strasser wrote:
> 64 bpp half float formats are supported on hdr planes only and are subject
> to the following restrictions:
>   * 90/270 rotation not supported
>   * Yf Tiling not supported
>   * Frame Buffer Compression not supported
>   * Color Keying not supported
> 
> v2:
> - Drop handling pixel normalize register
> - Don't use icl_is_hdr_plane too early
> 
> Signed-off-by: Kevin Strasser <kevin.strasser@intel.com>
> Cc: Uma Shankar <uma.shankar@intel.com>
> Cc: Shashank Sharma <shashank.sharma@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: dri-devel@lists.freedesktop.org
> ---
>  drivers/gpu/drm/i915/intel_display.c | 22 ++++++++++++
>  drivers/gpu/drm/i915/intel_sprite.c  | 67 ++++++++++++++++++++++++++++++++----
>  2 files changed, 83 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a6d8985..f413ccd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2668,6 +2668,18 @@ int skl_format_to_fourcc(int format, bool rgb_order, bool alpha)
>  		return DRM_FORMAT_RGB565;
>  	case PLANE_CTL_FORMAT_NV12:
>  		return DRM_FORMAT_NV12;
> +	case PLANE_CTL_FORMAT_XRGB_16161616F:
> +		if (rgb_order) {
> +			if (alpha)
> +				return DRM_FORMAT_ABGR16161616F;
> +			else
> +				return DRM_FORMAT_XBGR16161616F;
> +		} else {
> +			if (alpha)
> +				return DRM_FORMAT_ARGB16161616F;
> +			else
> +				return DRM_FORMAT_XRGB16161616F;
> +		}
>  	default:
>  	case PLANE_CTL_FORMAT_XRGB_8888:
>  		if (rgb_order) {
> @@ -3566,6 +3578,12 @@ static u32 skl_plane_ctl_format(u32 pixel_format)
>  		return PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_VYUY;
>  	case DRM_FORMAT_NV12:
>  		return PLANE_CTL_FORMAT_NV12;
> +	case DRM_FORMAT_XBGR16161616F:
> +	case DRM_FORMAT_ABGR16161616F:
> +		return PLANE_CTL_FORMAT_XRGB_16161616F | PLANE_CTL_ORDER_RGBX;
> +	case DRM_FORMAT_XRGB16161616F:
> +	case DRM_FORMAT_ARGB16161616F:
> +		return PLANE_CTL_FORMAT_XRGB_16161616F;
>  	default:
>  		MISSING_CASE(pixel_format);
>  	}
> @@ -5069,6 +5087,10 @@ static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
>  	case DRM_FORMAT_UYVY:
>  	case DRM_FORMAT_VYUY:
>  	case DRM_FORMAT_NV12:
> +	case DRM_FORMAT_XBGR16161616F:
> +	case DRM_FORMAT_ABGR16161616F:
> +	case DRM_FORMAT_XRGB16161616F:
> +	case DRM_FORMAT_ARGB16161616F:
>  		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 cd42e81..97f9d05 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1450,8 +1450,6 @@ static int skl_plane_check_fb(const struct intel_crtc_state *crtc_state,
>  		/*
>  		 * 90/270 is not allowed with RGB64 16:16:16:16 and
>  		 * Indexed 8-bit. RGB 16-bit 5:6:5 is allowed gen11 onwards.
> -		 * TBD: Add RGB64 case once its added in supported format
> -		 * list.
>  		 */
>  		switch (fb->format->format) {
>  		case DRM_FORMAT_RGB565:
> @@ -1459,6 +1457,10 @@ static int skl_plane_check_fb(const struct intel_crtc_state *crtc_state,
>  				break;
>  			/* fall through */
>  		case DRM_FORMAT_C8:
> +		case DRM_FORMAT_XRGB16161616F:
> +		case DRM_FORMAT_XBGR16161616F:
> +		case DRM_FORMAT_ARGB16161616F:
> +		case DRM_FORMAT_ABGR16161616F:
>  			DRM_DEBUG_KMS("Unsupported pixel format %s for 90/270!\n",
>  				      drm_get_format_name(fb->format->format,
>  							  &format_name));
> @@ -1774,6 +1776,45 @@ static const u32 skl_planar_formats[] = {
>  	DRM_FORMAT_NV12,
>  };
>  
> +static const uint32_t icl_hdr_plane_formats[] = {

Please switch to u32 & co. We recently had a mass conversion in the
driver.

> +	DRM_FORMAT_C8,
> +	DRM_FORMAT_RGB565,
> +	DRM_FORMAT_XRGB8888,
> +	DRM_FORMAT_XBGR8888,
> +	DRM_FORMAT_ARGB8888,
> +	DRM_FORMAT_ABGR8888,
> +	DRM_FORMAT_XRGB2101010,
> +	DRM_FORMAT_XBGR2101010,

I just noticed that icl should also support alpha for 10bpc. But that's
not relevant for this patch.

> +	DRM_FORMAT_XRGB16161616F,
> +	DRM_FORMAT_XBGR16161616F,
> +	DRM_FORMAT_ARGB16161616F,
> +	DRM_FORMAT_ABGR16161616F,
> +	DRM_FORMAT_YUYV,
> +	DRM_FORMAT_YVYU,
> +	DRM_FORMAT_UYVY,
> +	DRM_FORMAT_VYUY,
> +};
> +
> +static const uint32_t icl_hdr_planar_formats[] = {
> +	DRM_FORMAT_C8,
> +	DRM_FORMAT_RGB565,
> +	DRM_FORMAT_XRGB8888,
> +	DRM_FORMAT_XBGR8888,
> +	DRM_FORMAT_ARGB8888,
> +	DRM_FORMAT_ABGR8888,
> +	DRM_FORMAT_XRGB2101010,
> +	DRM_FORMAT_XBGR2101010,
> +	DRM_FORMAT_XRGB16161616F,
> +	DRM_FORMAT_XBGR16161616F,
> +	DRM_FORMAT_ARGB16161616F,
> +	DRM_FORMAT_ABGR16161616F,
> +	DRM_FORMAT_YUYV,
> +	DRM_FORMAT_YVYU,
> +	DRM_FORMAT_UYVY,
> +	DRM_FORMAT_VYUY,
> +	DRM_FORMAT_NV12,
> +};
> +
>  static const u64 skl_plane_format_modifiers_noccs[] = {
>  	I915_FORMAT_MOD_Yf_TILED,
>  	I915_FORMAT_MOD_Y_TILED,
> @@ -1917,6 +1958,10 @@ static bool skl_plane_format_mod_supported(struct drm_plane *_plane,
>  			return true;
>  		/* fall through */
>  	case DRM_FORMAT_C8:
> +	case DRM_FORMAT_XBGR16161616F:
> +	case DRM_FORMAT_ABGR16161616F:
> +	case DRM_FORMAT_XRGB16161616F:
> +	case DRM_FORMAT_ARGB16161616F:
>  		if (modifier == DRM_FORMAT_MOD_LINEAR ||
>  		    modifier == I915_FORMAT_MOD_X_TILED ||
>  		    modifier == I915_FORMAT_MOD_Y_TILED)
> @@ -2053,11 +2098,21 @@ skl_universal_plane_create(struct drm_i915_private *dev_priv,
>  		plane->update_slave = icl_update_slave;
>  
>  	if (skl_plane_has_planar(dev_priv, pipe, plane_id)) {
> -		formats = skl_planar_formats;
> -		num_formats = ARRAY_SIZE(skl_planar_formats);
> +		if (INTEL_GEN(dev_priv) > 10 && plane_id < PLANE_SPRITE2) {

is_hdr_plane() is around now, please use it.

With that and the u32 this is
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +			formats = icl_hdr_planar_formats;
> +			num_formats = ARRAY_SIZE(icl_hdr_planar_formats);
> +		} else {
> +			formats = skl_planar_formats;
> +			num_formats = ARRAY_SIZE(skl_planar_formats);
> +		}
>  	} else {
> -		formats = skl_plane_formats;
> -		num_formats = ARRAY_SIZE(skl_plane_formats);
> +		if (INTEL_GEN(dev_priv) > 10 && plane_id < PLANE_SPRITE2) {
> +			formats = icl_hdr_plane_formats;
> +			num_formats = ARRAY_SIZE(icl_hdr_plane_formats);
> +		} else {
> +			formats = skl_plane_formats;
> +			num_formats = ARRAY_SIZE(skl_plane_formats);
> +		}
>  	}
>  
>  	plane->has_ccs = skl_plane_has_ccs(dev_priv, pipe, plane_id);
> -- 
> 2.7.4
Kevin Strasser Feb. 1, 2019, 9:38 p.m. UTC | #2
Ville Syrjälä wrote:
> > @@ -1774,6 +1776,45 @@ static const u32 skl_planar_formats[] = {
> >        DRM_FORMAT_NV12,
> >  };
> >
> > +static const uint32_t icl_hdr_plane_formats[] = {
>
> Please switch to u32 & co. We recently had a mass conversion in the
> driver.

Will do. Looks like the CI caught that too.

> >  static const u64 skl_plane_format_modifiers_noccs[] = {
> >        I915_FORMAT_MOD_Yf_TILED,
> >        I915_FORMAT_MOD_Y_TILED,
> > @@ -1917,6 +1958,10 @@ static bool skl_plane_format_mod_supported(struct
> > drm_plane *_plane,
> >                        return true;
> >                /* fall through */
> >        case DRM_FORMAT_C8:
> > +     case DRM_FORMAT_XBGR16161616F:
> > +     case DRM_FORMAT_ABGR16161616F:
> > +     case DRM_FORMAT_XRGB16161616F:
> > +     case DRM_FORMAT_ARGB16161616F:
> >                if (modifier == DRM_FORMAT_MOD_LINEAR ||
> >                    modifier == I915_FORMAT_MOD_X_TILED ||
> >                    modifier == I915_FORMAT_MOD_Y_TILED)
> > @@ -2053,11 +2098,21 @@ skl_universal_plane_create(struct drm_i915_private
> > *dev_priv,
> >                plane->update_slave = icl_update_slave;
> >
> >        if (skl_plane_has_planar(dev_priv, pipe, plane_id)) {
> > -             formats = skl_planar_formats;
> > -             num_formats = ARRAY_SIZE(skl_planar_formats);
> > +             if (INTEL_GEN(dev_priv) > 10 && plane_id < PLANE_SPRITE2) {
>
> is_hdr_plane() is around now, please use it.

I don't think I can use icl_is_hdr_plane here without some refactoring. It 
requires the plane->base to be initialized through drm_universal_plane_init, 
which depends on formats/num_formats pointers to be already set.

Thanks,
Kevin
Ville Syrjälä Feb. 4, 2019, 9:24 p.m. UTC | #3
On Fri, Feb 01, 2019 at 09:38:57PM +0000, Strasser, Kevin wrote:
> Ville Syrjälä wrote:
> > > @@ -1774,6 +1776,45 @@ static const u32 skl_planar_formats[] = {
> > >        DRM_FORMAT_NV12,
> > >  };
> > >
> > > +static const uint32_t icl_hdr_plane_formats[] = {
> >
> > Please switch to u32 & co. We recently had a mass conversion in the
> > driver.
> 
> Will do. Looks like the CI caught that too.
> 
> > >  static const u64 skl_plane_format_modifiers_noccs[] = {
> > >        I915_FORMAT_MOD_Yf_TILED,
> > >        I915_FORMAT_MOD_Y_TILED,
> > > @@ -1917,6 +1958,10 @@ static bool skl_plane_format_mod_supported(struct
> > > drm_plane *_plane,
> > >                        return true;
> > >                /* fall through */
> > >        case DRM_FORMAT_C8:
> > > +     case DRM_FORMAT_XBGR16161616F:
> > > +     case DRM_FORMAT_ABGR16161616F:
> > > +     case DRM_FORMAT_XRGB16161616F:
> > > +     case DRM_FORMAT_ARGB16161616F:
> > >                if (modifier == DRM_FORMAT_MOD_LINEAR ||
> > >                    modifier == I915_FORMAT_MOD_X_TILED ||
> > >                    modifier == I915_FORMAT_MOD_Y_TILED)
> > > @@ -2053,11 +2098,21 @@ skl_universal_plane_create(struct drm_i915_private
> > > *dev_priv,
> > >                plane->update_slave = icl_update_slave;
> > >
> > >        if (skl_plane_has_planar(dev_priv, pipe, plane_id)) {
> > > -             formats = skl_planar_formats;
> > > -             num_formats = ARRAY_SIZE(skl_planar_formats);
> > > +             if (INTEL_GEN(dev_priv) > 10 && plane_id < PLANE_SPRITE2) {
> >
> > is_hdr_plane() is around now, please use it.
> 
> I don't think I can use icl_is_hdr_plane here without some refactoring. It 
> requires the plane->base to be initialized through drm_universal_plane_init, 
> which depends on formats/num_formats pointers to be already set.

Hmm. We should probably just convert it into

icl_is_hdr_plane(struct drm_i915_private *dev_priv,
		 enum plane_id plane_id);
Kevin Strasser Feb. 4, 2019, 10:16 p.m. UTC | #4
Ville Syrjälä wrote:
> Kevin Strasser wrote:
> > Ville Syrjälä wrote:
> > > is_hdr_plane() is around now, please use it.
> >
> > I don't think I can use icl_is_hdr_plane here without some refactoring. It
> > requires the plane->base to be initialized through drm_universal_plane_init,
> > which depends on formats/num_formats pointers to be already set.
>
> Hmm. We should probably just convert it into
>
> icl_is_hdr_plane(struct drm_i915_private *dev_priv,
>                  enum plane_id plane_id);

That sounds reasonable, I'll include this in v3.

Thanks,
Kevin
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a6d8985..f413ccd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2668,6 +2668,18 @@  int skl_format_to_fourcc(int format, bool rgb_order, bool alpha)
 		return DRM_FORMAT_RGB565;
 	case PLANE_CTL_FORMAT_NV12:
 		return DRM_FORMAT_NV12;
+	case PLANE_CTL_FORMAT_XRGB_16161616F:
+		if (rgb_order) {
+			if (alpha)
+				return DRM_FORMAT_ABGR16161616F;
+			else
+				return DRM_FORMAT_XBGR16161616F;
+		} else {
+			if (alpha)
+				return DRM_FORMAT_ARGB16161616F;
+			else
+				return DRM_FORMAT_XRGB16161616F;
+		}
 	default:
 	case PLANE_CTL_FORMAT_XRGB_8888:
 		if (rgb_order) {
@@ -3566,6 +3578,12 @@  static u32 skl_plane_ctl_format(u32 pixel_format)
 		return PLANE_CTL_FORMAT_YUV422 | PLANE_CTL_YUV422_VYUY;
 	case DRM_FORMAT_NV12:
 		return PLANE_CTL_FORMAT_NV12;
+	case DRM_FORMAT_XBGR16161616F:
+	case DRM_FORMAT_ABGR16161616F:
+		return PLANE_CTL_FORMAT_XRGB_16161616F | PLANE_CTL_ORDER_RGBX;
+	case DRM_FORMAT_XRGB16161616F:
+	case DRM_FORMAT_ARGB16161616F:
+		return PLANE_CTL_FORMAT_XRGB_16161616F;
 	default:
 		MISSING_CASE(pixel_format);
 	}
@@ -5069,6 +5087,10 @@  static int skl_update_scaler_plane(struct intel_crtc_state *crtc_state,
 	case DRM_FORMAT_UYVY:
 	case DRM_FORMAT_VYUY:
 	case DRM_FORMAT_NV12:
+	case DRM_FORMAT_XBGR16161616F:
+	case DRM_FORMAT_ABGR16161616F:
+	case DRM_FORMAT_XRGB16161616F:
+	case DRM_FORMAT_ARGB16161616F:
 		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 cd42e81..97f9d05 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -1450,8 +1450,6 @@  static int skl_plane_check_fb(const struct intel_crtc_state *crtc_state,
 		/*
 		 * 90/270 is not allowed with RGB64 16:16:16:16 and
 		 * Indexed 8-bit. RGB 16-bit 5:6:5 is allowed gen11 onwards.
-		 * TBD: Add RGB64 case once its added in supported format
-		 * list.
 		 */
 		switch (fb->format->format) {
 		case DRM_FORMAT_RGB565:
@@ -1459,6 +1457,10 @@  static int skl_plane_check_fb(const struct intel_crtc_state *crtc_state,
 				break;
 			/* fall through */
 		case DRM_FORMAT_C8:
+		case DRM_FORMAT_XRGB16161616F:
+		case DRM_FORMAT_XBGR16161616F:
+		case DRM_FORMAT_ARGB16161616F:
+		case DRM_FORMAT_ABGR16161616F:
 			DRM_DEBUG_KMS("Unsupported pixel format %s for 90/270!\n",
 				      drm_get_format_name(fb->format->format,
 							  &format_name));
@@ -1774,6 +1776,45 @@  static const u32 skl_planar_formats[] = {
 	DRM_FORMAT_NV12,
 };
 
+static const uint32_t icl_hdr_plane_formats[] = {
+	DRM_FORMAT_C8,
+	DRM_FORMAT_RGB565,
+	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_XBGR8888,
+	DRM_FORMAT_ARGB8888,
+	DRM_FORMAT_ABGR8888,
+	DRM_FORMAT_XRGB2101010,
+	DRM_FORMAT_XBGR2101010,
+	DRM_FORMAT_XRGB16161616F,
+	DRM_FORMAT_XBGR16161616F,
+	DRM_FORMAT_ARGB16161616F,
+	DRM_FORMAT_ABGR16161616F,
+	DRM_FORMAT_YUYV,
+	DRM_FORMAT_YVYU,
+	DRM_FORMAT_UYVY,
+	DRM_FORMAT_VYUY,
+};
+
+static const uint32_t icl_hdr_planar_formats[] = {
+	DRM_FORMAT_C8,
+	DRM_FORMAT_RGB565,
+	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_XBGR8888,
+	DRM_FORMAT_ARGB8888,
+	DRM_FORMAT_ABGR8888,
+	DRM_FORMAT_XRGB2101010,
+	DRM_FORMAT_XBGR2101010,
+	DRM_FORMAT_XRGB16161616F,
+	DRM_FORMAT_XBGR16161616F,
+	DRM_FORMAT_ARGB16161616F,
+	DRM_FORMAT_ABGR16161616F,
+	DRM_FORMAT_YUYV,
+	DRM_FORMAT_YVYU,
+	DRM_FORMAT_UYVY,
+	DRM_FORMAT_VYUY,
+	DRM_FORMAT_NV12,
+};
+
 static const u64 skl_plane_format_modifiers_noccs[] = {
 	I915_FORMAT_MOD_Yf_TILED,
 	I915_FORMAT_MOD_Y_TILED,
@@ -1917,6 +1958,10 @@  static bool skl_plane_format_mod_supported(struct drm_plane *_plane,
 			return true;
 		/* fall through */
 	case DRM_FORMAT_C8:
+	case DRM_FORMAT_XBGR16161616F:
+	case DRM_FORMAT_ABGR16161616F:
+	case DRM_FORMAT_XRGB16161616F:
+	case DRM_FORMAT_ARGB16161616F:
 		if (modifier == DRM_FORMAT_MOD_LINEAR ||
 		    modifier == I915_FORMAT_MOD_X_TILED ||
 		    modifier == I915_FORMAT_MOD_Y_TILED)
@@ -2053,11 +2098,21 @@  skl_universal_plane_create(struct drm_i915_private *dev_priv,
 		plane->update_slave = icl_update_slave;
 
 	if (skl_plane_has_planar(dev_priv, pipe, plane_id)) {
-		formats = skl_planar_formats;
-		num_formats = ARRAY_SIZE(skl_planar_formats);
+		if (INTEL_GEN(dev_priv) > 10 && plane_id < PLANE_SPRITE2) {
+			formats = icl_hdr_planar_formats;
+			num_formats = ARRAY_SIZE(icl_hdr_planar_formats);
+		} else {
+			formats = skl_planar_formats;
+			num_formats = ARRAY_SIZE(skl_planar_formats);
+		}
 	} else {
-		formats = skl_plane_formats;
-		num_formats = ARRAY_SIZE(skl_plane_formats);
+		if (INTEL_GEN(dev_priv) > 10 && plane_id < PLANE_SPRITE2) {
+			formats = icl_hdr_plane_formats;
+			num_formats = ARRAY_SIZE(icl_hdr_plane_formats);
+		} else {
+			formats = skl_plane_formats;
+			num_formats = ARRAY_SIZE(skl_plane_formats);
+		}
 	}
 
 	plane->has_ccs = skl_plane_has_ccs(dev_priv, pipe, plane_id);