[07/10] drm/i915/skl: Don't advertize ARGB2101010 and ABGR2101010
diff mbox

Message ID 1431443602-22886-8-git-send-email-damien.lespiau@intel.com
State New
Headers show

Commit Message

Lespiau, Damien May 12, 2015, 3:13 p.m. UTC
While we have been historically exposing those formats, but the hardware
doesn't do anything with the alpha bits. The DDX doesn't seem to
particularly care about those formats either, so just expose the
XRGB/XBGR variants.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

Comments

Daniel Vetter May 12, 2015, 4:13 p.m. UTC | #1
On Tue, May 12, 2015 at 04:13:19PM +0100, Damien Lespiau wrote:
> While we have been historically exposing those formats, but the hardware
> doesn't do anything with the alpha bits. The DDX doesn't seem to
> particularly care about those formats either, so just expose the
> XRGB/XBGR variants.
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

We have this problem with all the primary planes. Imo we should remove
this everywhere, since the ddx doesn't seem to care.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8adf3bf..13efade 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -70,6 +70,15 @@ static const uint32_t gen4_primary_formats[] = {
>  	DRM_FORMAT_ABGR2101010,
>  };
>  
> +
> +static const uint32_t skl_primary_formats[] = {
> +	COMMON_PRIMARY_FORMATS, \
> +	DRM_FORMAT_XBGR8888,
> +	DRM_FORMAT_ABGR8888,
> +	DRM_FORMAT_XRGB2101010,
> +	DRM_FORMAT_XBGR2101010,
> +};
> +
>  /* Cursor formats */
>  static const uint32_t intel_cursor_formats[] = {
>  	DRM_FORMAT_ARGB8888,
> @@ -13287,12 +13296,15 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
>  	if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4)
>  		primary->plane = !pipe;
>  
> -	if (INTEL_INFO(dev)->gen <= 3) {
> -		intel_primary_formats = gen2_primary_formats;
> -		num_formats = ARRAY_SIZE(gen2_primary_formats);
> -	} else {
> +	if (INTEL_INFO(dev)->gen >= 9) {
> +		intel_primary_formats = skl_primary_formats;
> +		num_formats = ARRAY_SIZE(skl_primary_formats);
> +	} else if (INTEL_INFO(dev)->gen >= 4) {
>  		intel_primary_formats = gen4_primary_formats;
>  		num_formats = ARRAY_SIZE(gen4_primary_formats);
> +	} else {
> +		intel_primary_formats = gen2_primary_formats;
> +		num_formats = ARRAY_SIZE(gen2_primary_formats);
>  	}
>  
>  	drm_universal_plane_init(dev, &primary->base, 0,
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Lespiau, Damien May 12, 2015, 4:18 p.m. UTC | #2
On Tue, May 12, 2015 at 06:13:25PM +0200, Daniel Vetter wrote:
> On Tue, May 12, 2015 at 04:13:19PM +0100, Damien Lespiau wrote:
> > While we have been historically exposing those formats, but the hardware
> > doesn't do anything with the alpha bits. The DDX doesn't seem to
> > particularly care about those formats either, so just expose the
> > XRGB/XBGR variants.
> > 
> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> 
> We have this problem with all the primary planes. Imo we should remove
> this everywhere, since the ddx doesn't seem to care.

Well technically that'd be breaking ABI (very unlikely, but still). So
I'd rather have this SKL-specific patch (because we haven't committed to
SKL just yet) separate from the patch removing the formats for previous
platforms in case we need to revert. I'll cook the patch for previous
platform as a follow up.
Ville Syrjälä May 12, 2015, 4:29 p.m. UTC | #3
On Tue, May 12, 2015 at 04:13:19PM +0100, Damien Lespiau wrote:
> While we have been historically exposing those formats, but the hardware
> doesn't do anything with the alpha bits. The DDX doesn't seem to
> particularly care about those formats either, so just expose the
> XRGB/XBGR variants.
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8adf3bf..13efade 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -70,6 +70,15 @@ static const uint32_t gen4_primary_formats[] = {
>  	DRM_FORMAT_ABGR2101010,
>  };
>  
> +
> +static const uint32_t skl_primary_formats[] = {
> +	COMMON_PRIMARY_FORMATS, \

Please kill COMMON_PRIMARY_FORMATS. It's making it hard to figure out
which formats are actually listed here.

> +	DRM_FORMAT_XBGR8888,
> +	DRM_FORMAT_ABGR8888,
> +	DRM_FORMAT_XRGB2101010,
> +	DRM_FORMAT_XBGR2101010,
> +};
> +
>  /* Cursor formats */
>  static const uint32_t intel_cursor_formats[] = {
>  	DRM_FORMAT_ARGB8888,
> @@ -13287,12 +13296,15 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
>  	if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4)
>  		primary->plane = !pipe;
>  
> -	if (INTEL_INFO(dev)->gen <= 3) {
> -		intel_primary_formats = gen2_primary_formats;
> -		num_formats = ARRAY_SIZE(gen2_primary_formats);
> -	} else {
> +	if (INTEL_INFO(dev)->gen >= 9) {
> +		intel_primary_formats = skl_primary_formats;
> +		num_formats = ARRAY_SIZE(skl_primary_formats);
> +	} else if (INTEL_INFO(dev)->gen >= 4) {
>  		intel_primary_formats = gen4_primary_formats;
>  		num_formats = ARRAY_SIZE(gen4_primary_formats);
> +	} else {
> +		intel_primary_formats = gen2_primary_formats;
> +		num_formats = ARRAY_SIZE(gen2_primary_formats);
>  	}
>  
>  	drm_universal_plane_init(dev, &primary->base, 0,
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Daniel Vetter May 12, 2015, 4:48 p.m. UTC | #4
On Tue, May 12, 2015 at 05:18:47PM +0100, Damien Lespiau wrote:
> On Tue, May 12, 2015 at 06:13:25PM +0200, Daniel Vetter wrote:
> > On Tue, May 12, 2015 at 04:13:19PM +0100, Damien Lespiau wrote:
> > > While we have been historically exposing those formats, but the hardware
> > > doesn't do anything with the alpha bits. The DDX doesn't seem to
> > > particularly care about those formats either, so just expose the
> > > XRGB/XBGR variants.
> > > 
> > > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> > 
> > We have this problem with all the primary planes. Imo we should remove
> > this everywhere, since the ddx doesn't seem to care.
> 
> Well technically that'd be breaking ABI (very unlikely, but still). So
> I'd rather have this SKL-specific patch (because we haven't committed to
> SKL just yet) separate from the patch removing the formats for previous
> platforms in case we need to revert. I'll cook the patch for previous
> platform as a follow up.

It's only breaking ABI if someone notices, which is wiggle room we exploit
a _lot_ to fix up past mistakes. I'm ok with merging the skl and pre-skl
parts as two patches in case of reverting, but I really think we should do
them all in one go.
-Daniel

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8adf3bf..13efade 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -70,6 +70,15 @@  static const uint32_t gen4_primary_formats[] = {
 	DRM_FORMAT_ABGR2101010,
 };
 
+
+static const uint32_t skl_primary_formats[] = {
+	COMMON_PRIMARY_FORMATS, \
+	DRM_FORMAT_XBGR8888,
+	DRM_FORMAT_ABGR8888,
+	DRM_FORMAT_XRGB2101010,
+	DRM_FORMAT_XBGR2101010,
+};
+
 /* Cursor formats */
 static const uint32_t intel_cursor_formats[] = {
 	DRM_FORMAT_ARGB8888,
@@ -13287,12 +13296,15 @@  static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
 	if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4)
 		primary->plane = !pipe;
 
-	if (INTEL_INFO(dev)->gen <= 3) {
-		intel_primary_formats = gen2_primary_formats;
-		num_formats = ARRAY_SIZE(gen2_primary_formats);
-	} else {
+	if (INTEL_INFO(dev)->gen >= 9) {
+		intel_primary_formats = skl_primary_formats;
+		num_formats = ARRAY_SIZE(skl_primary_formats);
+	} else if (INTEL_INFO(dev)->gen >= 4) {
 		intel_primary_formats = gen4_primary_formats;
 		num_formats = ARRAY_SIZE(gen4_primary_formats);
+	} else {
+		intel_primary_formats = gen2_primary_formats;
+		num_formats = ARRAY_SIZE(gen2_primary_formats);
 	}
 
 	drm_universal_plane_init(dev, &primary->base, 0,