[PATCH] drm/vmwgfx: Make sure the dirty tracker is released on surfaces

[PATCH] drm/vmwgfx: Make sure the dirty tracker is released on surfaces

[Zack Rusin]

Free all the dirty trackers associated with the surface when the
parent framebuffer is destroyed.

The buffers backing framebuffers are explicitly made coherent. Code
separates buffers from surfaces. Buffers only require a dirty tracker
which is released during the framebuffer cleanup. Surfaces, which can
be coherent to begin with, require a dirty tracker in the backing buffer
and in the resource. Neither was properly cleaned up. Make sure they're
correctly cleaned up when the parent framebuffer is destroyed.

Fixes: 609023571ff2 ("drm/vmwgfx: Refactor cursor handling")
Cc: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Zack Rusin <zack.rusin@broadcom.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index d8937f3de514..339ed2ddb717 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -438,7 +438,20 @@ static void vmw_framebuffer_surface_destroy(struct drm_framebuffer *framebuffer)
 {
 	struct vmw_framebuffer_surface *vfbs =
 		vmw_framebuffer_to_vfbs(framebuffer);
+	struct vmw_bo *bo = vmw_user_object_buffer(&vfbs->uo);
+	struct vmw_surface *surf = vmw_user_object_surface(&vfbs->uo);
 
+	if (bo) {
+		vmw_bo_dirty_release(bo);
+		/*
+		 * bo->dirty is reference counted so it being NULL
+		 * means that the surface wasn't coherent to begin
+		 * with and so we have to free the dirty tracker
+		 * in the vmw_resource
+		 */
+		if (!bo->dirty && surf && surf->res.dirty)
+			surf->res.func->dirty_free(&surf->res);
+	}
 	drm_framebuffer_cleanup(framebuffer);
 	vmw_user_object_unref(&vfbs->uo);
 
-- 
2.45.2

Re: [PATCH] drm/vmwgfx: Make sure the dirty tracker is released on surfaces

[Ian Forbes]

[-- Attachment #1: Type: text/plain, Size: 63 bytes --]

Looks good.

Reviewed-by: Ian Forbes <ian.forbes@broadcom.com>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5414 bytes --]

Re: [PATCH] drm/vmwgfx: Make sure the dirty tracker is released on surfaces

[kernel test robot]

Hi Zack,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm-exynos/exynos-drm-next]
[also build test ERROR on linus/master v6.14-rc5 next-20250305]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Zack-Rusin/drm-vmwgfx-Make-sure-the-dirty-tracker-is-released-on-surfaces/20250305-025105
base:   https://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git exynos-drm-next
patch link:    https://lore.kernel.org/r/20250304184942.2127704-1-zack.rusin%40broadcom.com
patch subject: [PATCH] drm/vmwgfx: Make sure the dirty tracker is released on surfaces
config: arm64-randconfig-002-20250305 (https://download.01.org/0day-ci/archive/20250306/202503060226.Y7UvsIZE-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250306/202503060226.Y7UvsIZE-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503060226.Y7UvsIZE-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/gpu/drm/vmwgfx/vmwgfx_kms.c: In function 'vmw_framebuffer_surface_destroy':
>> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c:1236:39: error: invalid use of undefined type 'const struct vmw_res_func'
    1236 |                         surf->res.func->dirty_free(&surf->res);
         |                                       ^~


vim +1236 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c

  1196	
  1197	
  1198	/**
  1199	 * vmw_du_connector_destroy_state - destroy connector state
  1200	 * @connector: DRM connector
  1201	 * @state: state object to destroy
  1202	 *
  1203	 * Destroys the connector state (both common and vmw-specific) for the
  1204	 * specified plane.
  1205	 */
  1206	void
  1207	vmw_du_connector_destroy_state(struct drm_connector *connector,
  1208				  struct drm_connector_state *state)
  1209	{
  1210		drm_atomic_helper_connector_destroy_state(connector, state);
  1211	}
  1212	/*
  1213	 * Generic framebuffer code
  1214	 */
  1215	
  1216	/*
  1217	 * Surface framebuffer code
  1218	 */
  1219	
  1220	static void vmw_framebuffer_surface_destroy(struct drm_framebuffer *framebuffer)
  1221	{
  1222		struct vmw_framebuffer_surface *vfbs =
  1223			vmw_framebuffer_to_vfbs(framebuffer);
  1224		struct vmw_bo *bo = vmw_user_object_buffer(&vfbs->uo);
  1225		struct vmw_surface *surf = vmw_user_object_surface(&vfbs->uo);
  1226	
  1227		if (bo) {
  1228			vmw_bo_dirty_release(bo);
  1229			/*
  1230			 * bo->dirty is reference counted so it being NULL
  1231			 * means that the surface wasn't coherent to begin
  1232			 * with and so we have to free the dirty tracker
  1233			 * in the vmw_resource
  1234			 */
  1235			if (!bo->dirty && surf && surf->res.dirty)
> 1236				surf->res.func->dirty_free(&surf->res);
  1237		}
  1238		drm_framebuffer_cleanup(framebuffer);
  1239		vmw_user_object_unref(&vfbs->uo);
  1240	
  1241		kfree(vfbs);
  1242	}
  1243	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] drm/vmwgfx: Make sure the dirty tracker is released on surfaces

[kernel test robot]

Hi Zack,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm-exynos/exynos-drm-next]
[also build test ERROR on linus/master v6.14-rc5 next-20250305]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Zack-Rusin/drm-vmwgfx-Make-sure-the-dirty-tracker-is-released-on-surfaces/20250305-025105
base:   https://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git exynos-drm-next
patch link:    https://lore.kernel.org/r/20250304184942.2127704-1-zack.rusin%40broadcom.com
patch subject: [PATCH] drm/vmwgfx: Make sure the dirty tracker is released on surfaces
config: arm64-allmodconfig (https://download.01.org/0day-ci/archive/20250306/202503060223.BrfYSoFw-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250306/202503060223.BrfYSoFw-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503060223.BrfYSoFw-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c:1236:18: error: incomplete definition of type 'struct vmw_res_func'
    1236 |                         surf->res.func->dirty_free(&surf->res);
         |                         ~~~~~~~~~~~~~~^
   drivers/gpu/drm/vmwgfx/vmwgfx_drv.h:120:8: note: forward declaration of 'struct vmw_res_func'
     120 | struct vmw_res_func;
         |        ^
   1 error generated.


vim +1236 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c

  1196	
  1197	
  1198	/**
  1199	 * vmw_du_connector_destroy_state - destroy connector state
  1200	 * @connector: DRM connector
  1201	 * @state: state object to destroy
  1202	 *
  1203	 * Destroys the connector state (both common and vmw-specific) for the
  1204	 * specified plane.
  1205	 */
  1206	void
  1207	vmw_du_connector_destroy_state(struct drm_connector *connector,
  1208				  struct drm_connector_state *state)
  1209	{
  1210		drm_atomic_helper_connector_destroy_state(connector, state);
  1211	}
  1212	/*
  1213	 * Generic framebuffer code
  1214	 */
  1215	
  1216	/*
  1217	 * Surface framebuffer code
  1218	 */
  1219	
  1220	static void vmw_framebuffer_surface_destroy(struct drm_framebuffer *framebuffer)
  1221	{
  1222		struct vmw_framebuffer_surface *vfbs =
  1223			vmw_framebuffer_to_vfbs(framebuffer);
  1224		struct vmw_bo *bo = vmw_user_object_buffer(&vfbs->uo);
  1225		struct vmw_surface *surf = vmw_user_object_surface(&vfbs->uo);
  1226	
  1227		if (bo) {
  1228			vmw_bo_dirty_release(bo);
  1229			/*
  1230			 * bo->dirty is reference counted so it being NULL
  1231			 * means that the surface wasn't coherent to begin
  1232			 * with and so we have to free the dirty tracker
  1233			 * in the vmw_resource
  1234			 */
  1235			if (!bo->dirty && surf && surf->res.dirty)
> 1236				surf->res.func->dirty_free(&surf->res);
  1237		}
  1238		drm_framebuffer_cleanup(framebuffer);
  1239		vmw_user_object_unref(&vfbs->uo);
  1240	
  1241		kfree(vfbs);
  1242	}
  1243	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] drm/vmwgfx: Make sure the dirty tracker is released on surfaces

[Zack Rusin]

[-- Attachment #1: Type: text/plain, Size: 918 bytes --]

On Wed, Mar 5, 2025 at 2:10 PM kernel test robot <lkp@intel.com> wrote:
>
> Hi Zack,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on drm-exynos/exynos-drm-next]
> [also build test ERROR on linus/master v6.14-rc5 next-20250305]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]

I just realized that the commit from Fixes hasn't been pushed to
drm-misc so the kernel test robots were running this patch without
applying the one this one fixes (because that sha doesn't exist in the
open), hence the build error.

Since we haven't pushed the commit from fixes to drm-misc, I'm going
to squash this change with the original one and will resend the
original one for another review instead.

z

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5414 bytes --]