[Intel-gfx] [PATCH 0/5] Check for valid framebuffer's format

[Intel-gfx] [PATCH 0/5] Check for valid framebuffer's format

[Maíra Canal]

This series is a follow-up of [1] in which I introduced a check for valid
formats on drm_gem_fb_create(). During the discussion, I realized that would be
a better idea to put the check inside framebuffer_check() so that it wouldn't
be needed to hit any driver-specific code path when the check fails. Thanks
to Daniel, Simon, Rob and Thomas for the insightful discussion!

Therefore, add the valid format check inside framebuffer_check() and remove
the same check from the drivers. Adding the check to framebuffer_check() will
guarantee that the igt@kms_addfb_basic@addfb25-bad-modifier IGT test passes,
showing the correct behavior of the check.

This patchset was tested on i915, amdgpu, and vc4 with the IGT tests.

[1] https://lore.kernel.org/dri-devel/20230103125322.855089-1-mcanal@igalia.com/T/

Best Regards,
- Maíra Canal

Maíra Canal (5):
  drm/framebuffer: Check for valid formats
  drm/amdgpu: Remove redundant framebuffer format check
  drm/i915: Remove redundant framebuffer format check
  drm/vmwgfx: Remove redundant framebuffer format check
  drm/plane: Unexport drm_any_plane_has_format()

 Documentation/gpu/todo.rst                  |  9 ++++-----
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 10 ----------
 drivers/gpu/drm/drm_framebuffer.c           |  8 ++++++++
 drivers/gpu/drm/drm_plane.c                 |  1 -
 drivers/gpu/drm/i915/display/intel_fb.c     |  9 ---------
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c         | 18 ------------------
 6 files changed, 12 insertions(+), 43 deletions(-)

-- 
2.39.0

[Intel-gfx] [PATCH 1/5] drm/framebuffer: Check for valid formats

[Maíra Canal]

Currently, framebuffer_check() doesn't check if the pixel format is
supported, which can lead to the acceptance of invalid pixel formats
e.g. the acceptance of invalid modifiers. Therefore, add a check for
valid formats on framebuffer_check(), so that the ADDFB2 IOCTL rejects
calls with invalid formats.

Moreover, note that this check is only valid for atomic drivers,
because, for non-atomic drivers, checking drm_any_plane_has_format() is
not possible since the format list for the primary plane is fake, and
we'd therefore reject valid formats.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 Documentation/gpu/todo.rst        | 9 ++++-----
 drivers/gpu/drm/drm_framebuffer.c | 8 ++++++++
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 1f8a5ebe188e..3a79c26c5cc7 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -276,11 +276,10 @@ Various hold-ups:
 - Need to switch to drm_fbdev_generic_setup(), otherwise a lot of the custom fb
   setup code can't be deleted.
 
-- Many drivers wrap drm_gem_fb_create() only to check for valid formats. For
-  atomic drivers we could check for valid formats by calling
-  drm_plane_check_pixel_format() against all planes, and pass if any plane
-  supports the format. For non-atomic that's not possible since like the format
-  list for the primary plane is fake and we'd therefor reject valid formats.
+- Need to switch to drm_gem_fb_create(), as now framebuffer_check() checks for
+  valid formats for atomic drivers.
+
+- Add an addfb format validation for non-atomic drivers.
 
 - Many drivers subclass drm_framebuffer, we'd need a embedding compatible
   version of the varios drm_gem_fb_create functions. Maybe called
diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
index aff3746dedfb..605642bf3650 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -280,6 +280,14 @@ static int framebuffer_check(struct drm_device *dev,
 		}
 	}
 
+	/* Verify that the modifier is supported. */
+	if (drm_drv_uses_atomic_modeset(dev) &&
+	    !drm_any_plane_has_format(dev, r->pixel_format, r->modifier[0])) {
+		drm_dbg_kms(dev, "Unsupported pixel format %p4cc / modifier 0x%llx\n",
+			    &r->pixel_format, r->modifier[0]);
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
-- 
2.39.0

[Intel-gfx] [PATCH 2/5] drm/amdgpu: Remove redundant framebuffer format check

[Maíra Canal]

Now that framebuffer_check() verifies that the format is properly
supported, there is no need to check it again on amdgpu's inside
helpers.

Therefore, remove the redundant framebuffer format check from the
amdgpu_display_gem_fb_verify_and_init() function, letting
framebuffer_check() perform the framebuffer validation.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index b22471b3bd63..611b7a4b086c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -1120,16 +1120,6 @@ static int amdgpu_display_gem_fb_verify_and_init(struct drm_device *dev,
 
 	rfb->base.obj[0] = obj;
 	drm_helper_mode_fill_fb_struct(dev, &rfb->base, mode_cmd);
-	/* Verify that the modifier is supported. */
-	if (!drm_any_plane_has_format(dev, mode_cmd->pixel_format,
-				      mode_cmd->modifier[0])) {
-		drm_dbg_kms(dev,
-			    "unsupported pixel format %p4cc / modifier 0x%llx\n",
-			    &mode_cmd->pixel_format, mode_cmd->modifier[0]);
-
-		ret = -EINVAL;
-		goto err;
-	}
 
 	ret = amdgpu_display_framebuffer_init(dev, rfb, mode_cmd, obj);
 	if (ret)
-- 
2.39.0

[Intel-gfx] [PATCH 3/5] drm/i915: Remove redundant framebuffer format check

[Maíra Canal]

Now that framebuffer_check() verifies that the format is properly
supported, there is no need to check it again on i915's inside
helpers.

Therefore, remove the redundant framebuffer format check from the
intel_framebuffer_init() function, letting framebuffer_check()
perform the framebuffer validation.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/i915/display/intel_fb.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c
index 63137ae5ab21..230b729e42d6 100644
--- a/drivers/gpu/drm/i915/display/intel_fb.c
+++ b/drivers/gpu/drm/i915/display/intel_fb.c
@@ -1914,15 +1914,6 @@ int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
 		}
 	}
 
-	if (!drm_any_plane_has_format(&dev_priv->drm,
-				      mode_cmd->pixel_format,
-				      mode_cmd->modifier[0])) {
-		drm_dbg_kms(&dev_priv->drm,
-			    "unsupported pixel format %p4cc / modifier 0x%llx\n",
-			    &mode_cmd->pixel_format, mode_cmd->modifier[0]);
-		goto err;
-	}
-
 	/*
 	 * gen2/3 display engine uses the fence if present,
 	 * so the tiling mode must match the fb modifier exactly.
-- 
2.39.0

[Intel-gfx] [PATCH 4/5] drm/vmwgfx: Remove redundant framebuffer format check

[Maíra Canal]

Now that framebuffer_check() verifies that the format is properly
supported, there is no need to check it again on vmwgfx's inside
helpers.

Therefore, remove the redundant framebuffer format check from the
vmw_kms_new_framebuffer_surface() and vmw_kms_new_framebuffer_bo()
functions, letting framebuffer_check() perform the framebuffer
validation.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 257f090071f1..05b8d8f912bf 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -1317,15 +1317,6 @@ static int vmw_kms_new_framebuffer_surface(struct vmw_private *dev_priv,
 	 * Sanity checks.
 	 */
 
-	if (!drm_any_plane_has_format(&dev_priv->drm,
-				      mode_cmd->pixel_format,
-				      mode_cmd->modifier[0])) {
-		drm_dbg(&dev_priv->drm,
-			"unsupported pixel format %p4cc / modifier 0x%llx\n",
-			&mode_cmd->pixel_format, mode_cmd->modifier[0]);
-		return -EINVAL;
-	}
-
 	/* Surface must be marked as a scanout. */
 	if (unlikely(!surface->metadata.scanout))
 		return -EINVAL;
@@ -1648,15 +1639,6 @@ static int vmw_kms_new_framebuffer_bo(struct vmw_private *dev_priv,
 		return -EINVAL;
 	}
 
-	if (!drm_any_plane_has_format(&dev_priv->drm,
-				      mode_cmd->pixel_format,
-				      mode_cmd->modifier[0])) {
-		drm_dbg(&dev_priv->drm,
-			"unsupported pixel format %p4cc / modifier 0x%llx\n",
-			&mode_cmd->pixel_format, mode_cmd->modifier[0]);
-		return -EINVAL;
-	}
-
 	vfbd = kzalloc(sizeof(*vfbd), GFP_KERNEL);
 	if (!vfbd) {
 		ret = -ENOMEM;
-- 
2.39.0

[Intel-gfx] [PATCH 5/5] drm/plane: Unexport drm_any_plane_has_format()

[Maíra Canal]

As the format validation is being dealt with exclusively
inside framebuffer_check(), there is no need to export the
drm_any_plane_has_format() symbol.

Therefore, unexport the drm_any_plane_has_format() symbol, reinforcing
that format validation is being dealt with by the DRM API.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/drm_plane.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 24e7998d1731..67c0ab60c7b6 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -837,7 +837,6 @@ bool drm_any_plane_has_format(struct drm_device *dev,
 
 	return false;
 }
-EXPORT_SYMBOL(drm_any_plane_has_format);
 
 /*
  * __setplane_internal - setplane handler for internal callers
-- 
2.39.0

Re: [Intel-gfx] [PATCH 1/5] drm/framebuffer: Check for valid formats

[Daniel Vetter]

On Mon, Jan 09, 2023 at 07:58:04AM -0300, Maíra Canal wrote:
> Currently, framebuffer_check() doesn't check if the pixel format is
> supported, which can lead to the acceptance of invalid pixel formats
> e.g. the acceptance of invalid modifiers. Therefore, add a check for
> valid formats on framebuffer_check(), so that the ADDFB2 IOCTL rejects
> calls with invalid formats.
> 
> Moreover, note that this check is only valid for atomic drivers,
> because, for non-atomic drivers, checking drm_any_plane_has_format() is
> not possible since the format list for the primary plane is fake, and
> we'd therefore reject valid formats.
> 
> Signed-off-by: Maíra Canal <mcanal@igalia.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  Documentation/gpu/todo.rst        | 9 ++++-----
>  drivers/gpu/drm/drm_framebuffer.c | 8 ++++++++
>  2 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 1f8a5ebe188e..3a79c26c5cc7 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -276,11 +276,10 @@ Various hold-ups:
>  - Need to switch to drm_fbdev_generic_setup(), otherwise a lot of the custom fb
>    setup code can't be deleted.
>  
> -- Many drivers wrap drm_gem_fb_create() only to check for valid formats. For
> -  atomic drivers we could check for valid formats by calling
> -  drm_plane_check_pixel_format() against all planes, and pass if any plane
> -  supports the format. For non-atomic that's not possible since like the format
> -  list for the primary plane is fake and we'd therefor reject valid formats.
> +- Need to switch to drm_gem_fb_create(), as now framebuffer_check() checks for
> +  valid formats for atomic drivers.
> +
> +- Add an addfb format validation for non-atomic drivers.
>  
>  - Many drivers subclass drm_framebuffer, we'd need a embedding compatible
>    version of the varios drm_gem_fb_create functions. Maybe called
> diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
> index aff3746dedfb..605642bf3650 100644
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -280,6 +280,14 @@ static int framebuffer_check(struct drm_device *dev,
>  		}
>  	}
>  
> +	/* Verify that the modifier is supported. */
> +	if (drm_drv_uses_atomic_modeset(dev) &&
> +	    !drm_any_plane_has_format(dev, r->pixel_format, r->modifier[0])) {
> +		drm_dbg_kms(dev, "Unsupported pixel format %p4cc / modifier 0x%llx\n",
> +			    &r->pixel_format, r->modifier[0]);
> +		return -EINVAL;
> +	}
> +
>  	return 0;
>  }
>  
> -- 
> 2.39.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [Intel-gfx] [PATCH 5/5] drm/plane: Unexport drm_any_plane_has_format()

[Daniel Vetter]

On Mon, Jan 09, 2023 at 07:58:08AM -0300, Maíra Canal wrote:
> As the format validation is being dealt with exclusively
> inside framebuffer_check(), there is no need to export the
> drm_any_plane_has_format() symbol.
> 
> Therefore, unexport the drm_any_plane_has_format() symbol, reinforcing
> that format validation is being dealt with by the DRM API.
> 
> Signed-off-by: Maíra Canal <mcanal@igalia.com>

Usually we also delete the kerneldoc at that point, since that's mainly
for driver authors and they don't need this anymore. With that

Also please move the function decl to the drm_crtc_internal.h since this
is no longer used outside of drm core/helper code.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/drm_plane.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 24e7998d1731..67c0ab60c7b6 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -837,7 +837,6 @@ bool drm_any_plane_has_format(struct drm_device *dev,
>  
>  	return false;
>  }
> -EXPORT_SYMBOL(drm_any_plane_has_format);
>  
>  /*
>   * __setplane_internal - setplane handler for internal callers
> -- 
> 2.39.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [Intel-gfx] [PATCH 4/5] drm/vmwgfx: Remove redundant framebuffer format check

[Zack Rusin]

On Mon, 2023-01-09 at 07:58 -0300, Maíra Canal wrote:
> Now that framebuffer_check() verifies that the format is properly
> supported, there is no need to check it again on vmwgfx's inside
> helpers.
> 
> Therefore, remove the redundant framebuffer format check from the
> vmw_kms_new_framebuffer_surface() and vmw_kms_new_framebuffer_bo()
> functions, letting framebuffer_check() perform the framebuffer
> validation.
> 
> Signed-off-by: Maíra Canal <mcanal@igalia.com>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 18 ------------------
>  1 file changed, 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index 257f090071f1..05b8d8f912bf 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -1317,15 +1317,6 @@ static int vmw_kms_new_framebuffer_surface(struct
> vmw_private *dev_priv,
>          * Sanity checks.
>          */
>  
> -       if (!drm_any_plane_has_format(&dev_priv->drm,
> -                                     mode_cmd->pixel_format,
> -                                     mode_cmd->modifier[0])) {
> -               drm_dbg(&dev_priv->drm,
> -                       "unsupported pixel format %p4cc / modifier 0x%llx\n",
> -                       &mode_cmd->pixel_format, mode_cmd->modifier[0]);
> -               return -EINVAL;
> -       }
> -
>         /* Surface must be marked as a scanout. */
>         if (unlikely(!surface->metadata.scanout))
>                 return -EINVAL;
> @@ -1648,15 +1639,6 @@ static int vmw_kms_new_framebuffer_bo(struct vmw_private
> *dev_priv,
>                 return -EINVAL;
>         }
>  
> -       if (!drm_any_plane_has_format(&dev_priv->drm,
> -                                     mode_cmd->pixel_format,
> -                                     mode_cmd->modifier[0])) {
> -               drm_dbg(&dev_priv->drm,
> -                       "unsupported pixel format %p4cc / modifier 0x%llx\n",
> -                       &mode_cmd->pixel_format, mode_cmd->modifier[0]);
> -               return -EINVAL;
> -       }
> -
>         vfbd = kzalloc(sizeof(*vfbd), GFP_KERNEL);
>         if (!vfbd) {
>                 ret = -ENOMEM;

That's a nice cleanup. Thanks.

Reviewed-by: Zack Rusin <zackr@vmware.com>

Re: [Intel-gfx] [PATCH 2/5] drm/amdgpu: Remove redundant framebuffer format check

[Simon Ser]

Reviewed-by: Simon Ser <contact@emersion.fr>

Re: [Intel-gfx] [PATCH 3/5] drm/i915: Remove redundant framebuffer format check

[Simon Ser]

The Intel counterpart is also:

Reviewed-by: Simon Ser <contact@emersion.fr>

Re: [Intel-gfx] [PATCH 3/5] drm/i915: Remove redundant framebuffer format check

[Ville Syrjälä]

On Mon, Jan 09, 2023 at 07:58:06AM -0300, Maíra Canal wrote:
> Now that framebuffer_check() verifies that the format is properly
> supported, there is no need to check it again on i915's inside
> helpers.
> 
> Therefore, remove the redundant framebuffer format check from the
> intel_framebuffer_init() function, letting framebuffer_check()
> perform the framebuffer validation.
> 
> Signed-off-by: Maíra Canal <mcanal@igalia.com>
> ---
>  drivers/gpu/drm/i915/display/intel_fb.c | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c
> index 63137ae5ab21..230b729e42d6 100644
> --- a/drivers/gpu/drm/i915/display/intel_fb.c
> +++ b/drivers/gpu/drm/i915/display/intel_fb.c
> @@ -1914,15 +1914,6 @@ int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
>  		}
>  	}
>  
> -	if (!drm_any_plane_has_format(&dev_priv->drm,
> -				      mode_cmd->pixel_format,
> -				      mode_cmd->modifier[0])) {
> -		drm_dbg_kms(&dev_priv->drm,
> -			    "unsupported pixel format %p4cc / modifier 0x%llx\n",
> -			    &mode_cmd->pixel_format, mode_cmd->modifier[0]);
> -		goto err;
> -	}
> -

This doesn't work for the legacy tiling->modifier path.

>  	/*
>  	 * gen2/3 display engine uses the fence if present,
>  	 * so the tiling mode must match the fb modifier exactly.
> -- 
> 2.39.0

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 3/5] drm/i915: Remove redundant framebuffer format check

[Maíra Canal]

Hi,

On 1/12/23 09:43, Ville Syrjälä wrote:
> On Mon, Jan 09, 2023 at 07:58:06AM -0300, Maíra Canal wrote:
>> Now that framebuffer_check() verifies that the format is properly
>> supported, there is no need to check it again on i915's inside
>> helpers.
>>
>> Therefore, remove the redundant framebuffer format check from the
>> intel_framebuffer_init() function, letting framebuffer_check()
>> perform the framebuffer validation.
>>
>> Signed-off-by: Maíra Canal <mcanal@igalia.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_fb.c | 9 ---------
>>   1 file changed, 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c
>> index 63137ae5ab21..230b729e42d6 100644
>> --- a/drivers/gpu/drm/i915/display/intel_fb.c
>> +++ b/drivers/gpu/drm/i915/display/intel_fb.c
>> @@ -1914,15 +1914,6 @@ int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
>>   		}
>>   	}
>>   
>> -	if (!drm_any_plane_has_format(&dev_priv->drm,
>> -				      mode_cmd->pixel_format,
>> -				      mode_cmd->modifier[0])) {
>> -		drm_dbg_kms(&dev_priv->drm,
>> -			    "unsupported pixel format %p4cc / modifier 0x%llx\n",
>> -			    &mode_cmd->pixel_format, mode_cmd->modifier[0]);
>> -		goto err;
>> -	}
>> -
> 
> This doesn't work for the legacy tiling->modifier path.

Do you have any idea on how we could remove drm_any_plane_has_format() from
i915? Or is it strictly necessary to validate the modifier in the legacy
path?

Best Regards,
- Maíra Canal

> 
>>   	/*
>>   	 * gen2/3 display engine uses the fence if present,
>>   	 * so the tiling mode must match the fb modifier exactly.
>> -- 
>> 2.39.0
> 

Re: [Intel-gfx] [PATCH 3/5] drm/i915: Remove redundant framebuffer format check

[Ville Syrjälä]

On Thu, Jan 12, 2023 at 11:07:59AM -0300, Maíra Canal wrote:
> Hi,
> 
> On 1/12/23 09:43, Ville Syrjälä wrote:
> > On Mon, Jan 09, 2023 at 07:58:06AM -0300, Maíra Canal wrote:
> >> Now that framebuffer_check() verifies that the format is properly
> >> supported, there is no need to check it again on i915's inside
> >> helpers.
> >>
> >> Therefore, remove the redundant framebuffer format check from the
> >> intel_framebuffer_init() function, letting framebuffer_check()
> >> perform the framebuffer validation.
> >>
> >> Signed-off-by: Maíra Canal <mcanal@igalia.com>
> >> ---
> >>   drivers/gpu/drm/i915/display/intel_fb.c | 9 ---------
> >>   1 file changed, 9 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_fb.c b/drivers/gpu/drm/i915/display/intel_fb.c
> >> index 63137ae5ab21..230b729e42d6 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_fb.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_fb.c
> >> @@ -1914,15 +1914,6 @@ int intel_framebuffer_init(struct intel_framebuffer *intel_fb,
> >>   		}
> >>   	}
> >>   
> >> -	if (!drm_any_plane_has_format(&dev_priv->drm,
> >> -				      mode_cmd->pixel_format,
> >> -				      mode_cmd->modifier[0])) {
> >> -		drm_dbg_kms(&dev_priv->drm,
> >> -			    "unsupported pixel format %p4cc / modifier 0x%llx\n",
> >> -			    &mode_cmd->pixel_format, mode_cmd->modifier[0]);
> >> -		goto err;
> >> -	}
> >> -
> > 
> > This doesn't work for the legacy tiling->modifier path.
> 
> Do you have any idea on how we could remove drm_any_plane_has_format() from
> i915? Or is it strictly necessary to validate the modifier in the legacy
> path?

I guess techinically we could skip it by knowing that X-tile is always
supported. However that may not hold in the future so not a soution I
really like. Also the drm_any_plane_has_format() call from 
framebuffer_check() is too early, so instead of checking X-tile
vs. linear based on the tiling it's going to always assume linear.

-- 
Ville Syrjälä
Intel