diff mbox series

[v2] drm/vmwgfx: Cleanup kms setup without 3d

Message ID 20240827043905.472825-1-zack.rusin@broadcom.com (mailing list archive)
State New, archived
Headers show
Series [v2] drm/vmwgfx: Cleanup kms setup without 3d | expand

Commit Message

Zack Rusin Aug. 27, 2024, 4:39 a.m. UTC
Do not validate format equality for the non 3d cases to allow xrgb to
argb copies and make sure the dx binding flags are only used
on dx compatible surfaces.

Fixes basic 2d kms setup on configurations without 3d. There's little
practical benefit to it because kms framebuffer coherence is disabled
on configurations without 3d but with those changes the code actually
makes sense.

v2: Remove the now unused format variable

Signed-off-by: Zack Rusin <zack.rusin@broadcom.com>
Fixes: d6667f0ddf46 ("drm/vmwgfx: Fix handling of dumb buffers")
Cc: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
Cc: dri-devel@lists.freedesktop.org
Cc: <stable@vger.kernel.org> # v6.9+
Cc: Maaz Mombasawala <maaz.mombasawala@broadcom.com>
Cc: Martin Krastev <martin.krastev@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c     | 29 -------------------------
 drivers/gpu/drm/vmwgfx/vmwgfx_surface.c |  9 +++++---
 2 files changed, 6 insertions(+), 32 deletions(-)

Comments

Martin Krastev Aug. 27, 2024, 9:16 a.m. UTC | #1
On Tue, Aug 27, 2024 at 7:39 AM Zack Rusin <zack.rusin@broadcom.com> wrote:
>
> Do not validate format equality for the non 3d cases to allow xrgb to
> argb copies and make sure the dx binding flags are only used
> on dx compatible surfaces.
>
> Fixes basic 2d kms setup on configurations without 3d. There's little
> practical benefit to it because kms framebuffer coherence is disabled
> on configurations without 3d but with those changes the code actually
> makes sense.
>
> v2: Remove the now unused format variable
>
> Signed-off-by: Zack Rusin <zack.rusin@broadcom.com>
> Fixes: d6667f0ddf46 ("drm/vmwgfx: Fix handling of dumb buffers")
> Cc: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: <stable@vger.kernel.org> # v6.9+
> Cc: Maaz Mombasawala <maaz.mombasawala@broadcom.com>
> Cc: Martin Krastev <martin.krastev@broadcom.com>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c     | 29 -------------------------
>  drivers/gpu/drm/vmwgfx/vmwgfx_surface.c |  9 +++++---
>  2 files changed, 6 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index 288ed0bb75cb..282b6153bcdd 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -1283,7 +1283,6 @@ static int vmw_kms_new_framebuffer_surface(struct vmw_private *dev_priv,
>  {
>         struct drm_device *dev = &dev_priv->drm;
>         struct vmw_framebuffer_surface *vfbs;
> -       enum SVGA3dSurfaceFormat format;
>         struct vmw_surface *surface;
>         int ret;
>
> @@ -1320,34 +1319,6 @@ static int vmw_kms_new_framebuffer_surface(struct vmw_private *dev_priv,
>                 return -EINVAL;
>         }
>
> -       switch (mode_cmd->pixel_format) {
> -       case DRM_FORMAT_ARGB8888:
> -               format = SVGA3D_A8R8G8B8;
> -               break;
> -       case DRM_FORMAT_XRGB8888:
> -               format = SVGA3D_X8R8G8B8;
> -               break;
> -       case DRM_FORMAT_RGB565:
> -               format = SVGA3D_R5G6B5;
> -               break;
> -       case DRM_FORMAT_XRGB1555:
> -               format = SVGA3D_A1R5G5B5;
> -               break;
> -       default:
> -               DRM_ERROR("Invalid pixel format: %p4cc\n",
> -                         &mode_cmd->pixel_format);
> -               return -EINVAL;
> -       }
> -
> -       /*
> -        * For DX, surface format validation is done when surface->scanout
> -        * is set.
> -        */
> -       if (!has_sm4_context(dev_priv) && format != surface->metadata.format) {
> -               DRM_ERROR("Invalid surface format for requested mode.\n");
> -               return -EINVAL;
> -       }
> -
>         vfbs = kzalloc(sizeof(*vfbs), GFP_KERNEL);
>         if (!vfbs) {
>                 ret = -ENOMEM;
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> index 1625b30d9970..5721c74da3e0 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> @@ -2276,9 +2276,12 @@ int vmw_dumb_create(struct drm_file *file_priv,
>         const struct SVGA3dSurfaceDesc *desc = vmw_surface_get_desc(format);
>         SVGA3dSurfaceAllFlags flags = SVGA3D_SURFACE_HINT_TEXTURE |
>                                       SVGA3D_SURFACE_HINT_RENDERTARGET |
> -                                     SVGA3D_SURFACE_SCREENTARGET |
> -                                     SVGA3D_SURFACE_BIND_SHADER_RESOURCE |
> -                                     SVGA3D_SURFACE_BIND_RENDER_TARGET;
> +                                     SVGA3D_SURFACE_SCREENTARGET;
> +
> +       if (vmw_surface_is_dx_screen_target_format(format)) {
> +               flags |= SVGA3D_SURFACE_BIND_SHADER_RESOURCE |
> +                        SVGA3D_SURFACE_BIND_RENDER_TARGET;
> +       }
>
>         /*
>          * Without mob support we're just going to use raw memory buffer
> --
> 2.43.0
>

LGTM.

Reviewed-by: Martin Krastev <martin.krastev@broadcom.com>

Regards,
Martin
Maaz Mombasawala Aug. 29, 2024, 11:08 p.m. UTC | #2
On 8/26/24 21:39, Zack Rusin wrote:
> Do not validate format equality for the non 3d cases to allow xrgb to
> argb copies and make sure the dx binding flags are only used
> on dx compatible surfaces.
> 
> Fixes basic 2d kms setup on configurations without 3d. There's little
> practical benefit to it because kms framebuffer coherence is disabled
> on configurations without 3d but with those changes the code actually
> makes sense.
> 
> v2: Remove the now unused format variable
> 
> Signed-off-by: Zack Rusin <zack.rusin@broadcom.com>
> Fixes: d6667f0ddf46 ("drm/vmwgfx: Fix handling of dumb buffers")
> Cc: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
> Cc: dri-devel@lists.freedesktop.org
> Cc: <stable@vger.kernel.org> # v6.9+
> Cc: Maaz Mombasawala <maaz.mombasawala@broadcom.com>
> Cc: Martin Krastev <martin.krastev@broadcom.com>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c     | 29 -------------------------
>  drivers/gpu/drm/vmwgfx/vmwgfx_surface.c |  9 +++++---
>  2 files changed, 6 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index 288ed0bb75cb..282b6153bcdd 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -1283,7 +1283,6 @@ static int vmw_kms_new_framebuffer_surface(struct vmw_private *dev_priv,
>  {
>  	struct drm_device *dev = &dev_priv->drm;
>  	struct vmw_framebuffer_surface *vfbs;
> -	enum SVGA3dSurfaceFormat format;
>  	struct vmw_surface *surface;
>  	int ret;
>  
> @@ -1320,34 +1319,6 @@ static int vmw_kms_new_framebuffer_surface(struct vmw_private *dev_priv,
>  		return -EINVAL;
>  	}
>  
> -	switch (mode_cmd->pixel_format) {
> -	case DRM_FORMAT_ARGB8888:
> -		format = SVGA3D_A8R8G8B8;
> -		break;
> -	case DRM_FORMAT_XRGB8888:
> -		format = SVGA3D_X8R8G8B8;
> -		break;
> -	case DRM_FORMAT_RGB565:
> -		format = SVGA3D_R5G6B5;
> -		break;
> -	case DRM_FORMAT_XRGB1555:
> -		format = SVGA3D_A1R5G5B5;
> -		break;
> -	default:
> -		DRM_ERROR("Invalid pixel format: %p4cc\n",
> -			  &mode_cmd->pixel_format);
> -		return -EINVAL;
> -	}
> -
> -	/*
> -	 * For DX, surface format validation is done when surface->scanout
> -	 * is set.
> -	 */
> -	if (!has_sm4_context(dev_priv) && format != surface->metadata.format) {
> -		DRM_ERROR("Invalid surface format for requested mode.\n");
> -		return -EINVAL;
> -	}
> -
>  	vfbs = kzalloc(sizeof(*vfbs), GFP_KERNEL);
>  	if (!vfbs) {
>  		ret = -ENOMEM;
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> index 1625b30d9970..5721c74da3e0 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> @@ -2276,9 +2276,12 @@ int vmw_dumb_create(struct drm_file *file_priv,
>  	const struct SVGA3dSurfaceDesc *desc = vmw_surface_get_desc(format);
>  	SVGA3dSurfaceAllFlags flags = SVGA3D_SURFACE_HINT_TEXTURE |
>  				      SVGA3D_SURFACE_HINT_RENDERTARGET |
> -				      SVGA3D_SURFACE_SCREENTARGET |
> -				      SVGA3D_SURFACE_BIND_SHADER_RESOURCE |
> -				      SVGA3D_SURFACE_BIND_RENDER_TARGET;
> +				      SVGA3D_SURFACE_SCREENTARGET;
> +
> +	if (vmw_surface_is_dx_screen_target_format(format)) {
> +		flags |= SVGA3D_SURFACE_BIND_SHADER_RESOURCE |
> +			 SVGA3D_SURFACE_BIND_RENDER_TARGET;
> +	}
>  
>  	/*
>  	 * Without mob support we're just going to use raw memory buffer

LGTM

Reviewed-by: Maaz Mombasawala <maaz.mombasawala@broadcom.com>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 288ed0bb75cb..282b6153bcdd 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -1283,7 +1283,6 @@  static int vmw_kms_new_framebuffer_surface(struct vmw_private *dev_priv,
 {
 	struct drm_device *dev = &dev_priv->drm;
 	struct vmw_framebuffer_surface *vfbs;
-	enum SVGA3dSurfaceFormat format;
 	struct vmw_surface *surface;
 	int ret;
 
@@ -1320,34 +1319,6 @@  static int vmw_kms_new_framebuffer_surface(struct vmw_private *dev_priv,
 		return -EINVAL;
 	}
 
-	switch (mode_cmd->pixel_format) {
-	case DRM_FORMAT_ARGB8888:
-		format = SVGA3D_A8R8G8B8;
-		break;
-	case DRM_FORMAT_XRGB8888:
-		format = SVGA3D_X8R8G8B8;
-		break;
-	case DRM_FORMAT_RGB565:
-		format = SVGA3D_R5G6B5;
-		break;
-	case DRM_FORMAT_XRGB1555:
-		format = SVGA3D_A1R5G5B5;
-		break;
-	default:
-		DRM_ERROR("Invalid pixel format: %p4cc\n",
-			  &mode_cmd->pixel_format);
-		return -EINVAL;
-	}
-
-	/*
-	 * For DX, surface format validation is done when surface->scanout
-	 * is set.
-	 */
-	if (!has_sm4_context(dev_priv) && format != surface->metadata.format) {
-		DRM_ERROR("Invalid surface format for requested mode.\n");
-		return -EINVAL;
-	}
-
 	vfbs = kzalloc(sizeof(*vfbs), GFP_KERNEL);
 	if (!vfbs) {
 		ret = -ENOMEM;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
index 1625b30d9970..5721c74da3e0 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
@@ -2276,9 +2276,12 @@  int vmw_dumb_create(struct drm_file *file_priv,
 	const struct SVGA3dSurfaceDesc *desc = vmw_surface_get_desc(format);
 	SVGA3dSurfaceAllFlags flags = SVGA3D_SURFACE_HINT_TEXTURE |
 				      SVGA3D_SURFACE_HINT_RENDERTARGET |
-				      SVGA3D_SURFACE_SCREENTARGET |
-				      SVGA3D_SURFACE_BIND_SHADER_RESOURCE |
-				      SVGA3D_SURFACE_BIND_RENDER_TARGET;
+				      SVGA3D_SURFACE_SCREENTARGET;
+
+	if (vmw_surface_is_dx_screen_target_format(format)) {
+		flags |= SVGA3D_SURFACE_BIND_SHADER_RESOURCE |
+			 SVGA3D_SURFACE_BIND_RENDER_TARGET;
+	}
 
 	/*
 	 * Without mob support we're just going to use raw memory buffer