virtio_gpu_fence_release is added to free virtio-gpu-fence upon release of dma_fence.
Cc: Gurchetan Singh gurchetansingh@chromium.org Cc: Gerd Hoffmann kraxel@redhat.com Cc: Vivek Kasireddy vivek.kasireddy@intel.com Signed-off-by: Dongwon Kim dongwon.kim@intel.com --- drivers/gpu/drm/virtio/virtgpu_fence.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/virtio/virtgpu_fence.c b/drivers/gpu/drm/virtio/virtgpu_fence.c index f28357dbde35..ba659ac2a51d 100644 --- a/drivers/gpu/drm/virtio/virtgpu_fence.c +++ b/drivers/gpu/drm/virtio/virtgpu_fence.c @@ -63,12 +63,20 @@ static void virtio_gpu_timeline_value_str(struct dma_fence *f, char *str, (u64)atomic64_read(&fence->drv->last_fence_id)); }
+static void virtio_gpu_fence_release(struct dma_fence *f) +{ + struct virtio_gpu_fence *fence = to_virtio_gpu_fence(f); + + kfree(fence); +} + static const struct dma_fence_ops virtio_gpu_fence_ops = { .get_driver_name = virtio_gpu_get_driver_name, .get_timeline_name = virtio_gpu_get_timeline_name, .signaled = virtio_gpu_fence_signaled, .fence_value_str = virtio_gpu_fence_value_str, .timeline_value_str = virtio_gpu_timeline_value_str, + .release = virtio_gpu_fence_release, };
struct virtio_gpu_fence *virtio_gpu_fence_alloc(struct virtio_gpu_device *vgdev,
Having one fence for a vgfb would cause conflict in case there are multiple planes referencing the same vgfb (e.g. Xorg screen covering two displays in extended mode) being flushed simultaneously. So it makes sence to use a separated fence for each plane update to prevent this.
vgfb->fence is not required anymore with the suggested code change so both prepare_fb and cleanup_fb are removed since only fence creation/ freeing are done in there.
Cc: Gurchetan Singh gurchetansingh@chromium.org Cc: Gerd Hoffmann kraxel@redhat.com Cc: Vivek Kasireddy vivek.kasireddy@intel.com Signed-off-by: Dongwon Kim dongwon.kim@intel.com --- drivers/gpu/drm/virtio/virtgpu_drv.h | 1 - drivers/gpu/drm/virtio/virtgpu_plane.c | 98 +++++++++----------------- 2 files changed, 35 insertions(+), 64 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index 0a194aaad419..4c59c1e67ca5 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -186,7 +186,6 @@ struct virtio_gpu_output {
struct virtio_gpu_framebuffer { struct drm_framebuffer base; - struct virtio_gpu_fence *fence; }; #define to_virtio_gpu_framebuffer(x) \ container_of(x, struct virtio_gpu_framebuffer, base) diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c index 6d3cc9e238a4..9856e9941e37 100644 --- a/drivers/gpu/drm/virtio/virtgpu_plane.c +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c @@ -137,29 +137,36 @@ static void virtio_gpu_resource_flush(struct drm_plane *plane, struct virtio_gpu_device *vgdev = dev->dev_private; struct virtio_gpu_framebuffer *vgfb; struct virtio_gpu_object *bo; + struct virtio_gpu_object_array *objs; + struct virtio_gpu_fence *fence;
vgfb = to_virtio_gpu_framebuffer(plane->state->fb); bo = gem_to_virtio_gpu_obj(vgfb->base.obj[0]); - if (vgfb->fence) { - struct virtio_gpu_object_array *objs;
+ if (bo && bo->dumb && (plane->state->fb != new_state->fb) && + ((plane->type == DRM_PLANE_TYPE_PRIMARY && bo->guest_blob) || + plane->type != DRM_PLANE_TYPE_PRIMARY)) + fence = virtio_gpu_fence_alloc(vgdev, vgdev->fence_drv.context, + 0); + + if (fence) { objs = virtio_gpu_array_alloc(1); - if (!objs) + if (!objs) { + kfree(fence); return; + } virtio_gpu_array_add_obj(objs, vgfb->base.obj[0]); virtio_gpu_array_lock_resv(objs); - virtio_gpu_cmd_resource_flush(vgdev, bo->hw_res_handle, x, y, - width, height, objs, vgfb->fence); - virtio_gpu_notify(vgdev); + } + + virtio_gpu_cmd_resource_flush(vgdev, bo->hw_res_handle, x, y, + width, height, objs, fence); + virtio_gpu_notify(vgdev);
- dma_fence_wait_timeout(&vgfb->fence->f, true, + if (fence) { + dma_fence_wait_timeout(&fence->f, true, msecs_to_jiffies(50)); - dma_fence_put(&vgfb->fence->f); - vgfb->fence = NULL; - } else { - virtio_gpu_cmd_resource_flush(vgdev, bo->hw_res_handle, x, y, - width, height, NULL, NULL); - virtio_gpu_notify(vgdev); + dma_fence_put(&fence->f); } }
@@ -239,47 +246,6 @@ static void virtio_gpu_primary_plane_update(struct drm_plane *plane, rect.y2 - rect.y1); }
-static int virtio_gpu_plane_prepare_fb(struct drm_plane *plane, - struct drm_plane_state *new_state) -{ - struct drm_device *dev = plane->dev; - struct virtio_gpu_device *vgdev = dev->dev_private; - struct virtio_gpu_framebuffer *vgfb; - struct virtio_gpu_object *bo; - - if (!new_state->fb) - return 0; - - vgfb = to_virtio_gpu_framebuffer(new_state->fb); - bo = gem_to_virtio_gpu_obj(vgfb->base.obj[0]); - if (!bo || (plane->type == DRM_PLANE_TYPE_PRIMARY && !bo->guest_blob)) - return 0; - - if (bo->dumb && (plane->state->fb != new_state->fb)) { - vgfb->fence = virtio_gpu_fence_alloc(vgdev, vgdev->fence_drv.context, - 0); - if (!vgfb->fence) - return -ENOMEM; - } - - return 0; -} - -static void virtio_gpu_plane_cleanup_fb(struct drm_plane *plane, - struct drm_plane_state *old_state) -{ - struct virtio_gpu_framebuffer *vgfb; - - if (!plane->state->fb) - return; - - vgfb = to_virtio_gpu_framebuffer(plane->state->fb); - if (vgfb->fence) { - dma_fence_put(&vgfb->fence->f); - vgfb->fence = NULL; - } -} - static void virtio_gpu_cursor_plane_update(struct drm_plane *plane, struct drm_atomic_state *state) { @@ -310,21 +276,31 @@ static void virtio_gpu_cursor_plane_update(struct drm_plane *plane, if (bo && bo->dumb && (plane->state->fb != old_state->fb)) { /* new cursor -- update & wait */ struct virtio_gpu_object_array *objs; + struct virtio_gpu_fence *fence;
+ fence = virtio_gpu_fence_alloc(vgdev, vgdev->fence_drv.context, + 0); objs = virtio_gpu_array_alloc(1); - if (!objs) + if (!objs) { + if (fence) + kfree(fence); + return; + } + virtio_gpu_array_add_obj(objs, vgfb->base.obj[0]); virtio_gpu_array_lock_resv(objs); virtio_gpu_cmd_transfer_to_host_2d (vgdev, 0, plane->state->crtc_w, plane->state->crtc_h, - 0, 0, objs, vgfb->fence); + 0, 0, objs, fence); virtio_gpu_notify(vgdev); - dma_fence_wait(&vgfb->fence->f, true); - dma_fence_put(&vgfb->fence->f); - vgfb->fence = NULL; + + if (fence) { + dma_fence_wait(&fence->f, true); + dma_fence_put(&fence->f); + } }
if (plane->state->fb != old_state->fb) { @@ -358,15 +334,11 @@ static void virtio_gpu_cursor_plane_update(struct drm_plane *plane, }
static const struct drm_plane_helper_funcs virtio_gpu_primary_helper_funcs = { - .prepare_fb = virtio_gpu_plane_prepare_fb, - .cleanup_fb = virtio_gpu_plane_cleanup_fb, .atomic_check = virtio_gpu_plane_atomic_check, .atomic_update = virtio_gpu_primary_plane_update, };
static const struct drm_plane_helper_funcs virtio_gpu_cursor_helper_funcs = { - .prepare_fb = virtio_gpu_plane_prepare_fb, - .cleanup_fb = virtio_gpu_plane_cleanup_fb, .atomic_check = virtio_gpu_plane_atomic_check, .atomic_update = virtio_gpu_cursor_plane_update, };
Trying to use the fence to make plane update to wait for the host to consume the buffer for better synchronization in all cases
Cc: Gurchetan Singh gurchetansingh@chromium.org Cc: Gerd Hoffmann kraxel@redhat.com Cc: Vivek Kasireddy vivek.kasireddy@intel.com Signed-off-by: Dongwon Kim dongwon.kim@intel.com --- drivers/gpu/drm/virtio/virtgpu_plane.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c index 9856e9941e37..0333181e9dbf 100644 --- a/drivers/gpu/drm/virtio/virtgpu_plane.c +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c @@ -142,12 +142,7 @@ static void virtio_gpu_resource_flush(struct drm_plane *plane,
vgfb = to_virtio_gpu_framebuffer(plane->state->fb); bo = gem_to_virtio_gpu_obj(vgfb->base.obj[0]); - - if (bo && bo->dumb && (plane->state->fb != new_state->fb) && - ((plane->type == DRM_PLANE_TYPE_PRIMARY && bo->guest_blob) || - plane->type != DRM_PLANE_TYPE_PRIMARY)) - fence = virtio_gpu_fence_alloc(vgdev, vgdev->fence_drv.context, - 0); + fence = virtio_gpu_fence_alloc(vgdev, vgdev->fence_drv.context, 0);
if (fence) { objs = virtio_gpu_array_alloc(1);
Hi Dongwon,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm/drm-next] [also build test WARNING on drm-exynos/exynos-drm-next drm-intel/for-linux-next drm-tip/drm-tip tegra-drm/drm/tegra/for-next v5.18-rc6 next-20220510] [cannot apply to airlied/drm-next] [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]
url: https://github.com/intel-lab-lkp/linux/commits/Dongwon-Kim/drm-virtio-releas... base: git://anongit.freedesktop.org/drm/drm drm-next config: arm-randconfig-r015-20220509 (https://download.01.org/0day-ci/archive/20220511/202205111533.LaZm5cYP-lkp@i...) compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 18dd123c56754edf62c7042dcf23185c3727610f) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install arm cross compiling tool for clang build # apt-get install binutils-arm-linux-gnueabi # https://github.com/intel-lab-lkp/linux/commit/5c7787de5069b504754aea17bf8137... git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Dongwon-Kim/drm-virtio-release-ops-for-virtgpu-fence-release/20220511-081226 git checkout 5c7787de5069b504754aea17bf8137faae26fe66 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/gpu/drm/virtio/
If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com
All warnings (new ones prefixed by >>):
drivers/gpu/drm/virtio/virtgpu_plane.c:147:6: warning: variable 'objs' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
if (fence) { ^~~~~ drivers/gpu/drm/virtio/virtgpu_plane.c:158:26: note: uninitialized use occurs here width, height, objs, fence); ^~~~ drivers/gpu/drm/virtio/virtgpu_plane.c:147:2: note: remove the 'if' if its condition is always true if (fence) { ^~~~~~~~~~~ drivers/gpu/drm/virtio/virtgpu_plane.c:140:38: note: initialize the variable 'objs' to silence this warning struct virtio_gpu_object_array *objs; ^ = NULL 1 warning generated.
vim +147 drivers/gpu/drm/virtio/virtgpu_plane.c
544c521d4ab8f2a Gerd Hoffmann 2019-10-23 131 5c68ab9276aae47 Vivek Kasireddy 2021-06-10 132 static void virtio_gpu_resource_flush(struct drm_plane *plane, 5c68ab9276aae47 Vivek Kasireddy 2021-06-10 133 uint32_t x, uint32_t y, 5c68ab9276aae47 Vivek Kasireddy 2021-06-10 134 uint32_t width, uint32_t height) 5c68ab9276aae47 Vivek Kasireddy 2021-06-10 135 { 5c68ab9276aae47 Vivek Kasireddy 2021-06-10 136 struct drm_device *dev = plane->dev; 5c68ab9276aae47 Vivek Kasireddy 2021-06-10 137 struct virtio_gpu_device *vgdev = dev->dev_private; 5c68ab9276aae47 Vivek Kasireddy 2021-06-10 138 struct virtio_gpu_framebuffer *vgfb; 5c68ab9276aae47 Vivek Kasireddy 2021-06-10 139 struct virtio_gpu_object *bo; 47558b189c1d340 Dongwon Kim 2022-05-10 140 struct virtio_gpu_object_array *objs; 47558b189c1d340 Dongwon Kim 2022-05-10 141 struct virtio_gpu_fence *fence; 5c68ab9276aae47 Vivek Kasireddy 2021-06-10 142 5c68ab9276aae47 Vivek Kasireddy 2021-06-10 143 vgfb = to_virtio_gpu_framebuffer(plane->state->fb); 5c68ab9276aae47 Vivek Kasireddy 2021-06-10 144 bo = gem_to_virtio_gpu_obj(vgfb->base.obj[0]); 5c7787de5069b50 Dongwon Kim 2022-05-10 145 fence = virtio_gpu_fence_alloc(vgdev, vgdev->fence_drv.context, 0); 47558b189c1d340 Dongwon Kim 2022-05-10 146 47558b189c1d340 Dongwon Kim 2022-05-10 @147 if (fence) { 5c68ab9276aae47 Vivek Kasireddy 2021-06-10 148 objs = virtio_gpu_array_alloc(1); 47558b189c1d340 Dongwon Kim 2022-05-10 149 if (!objs) { 47558b189c1d340 Dongwon Kim 2022-05-10 150 kfree(fence); 5c68ab9276aae47 Vivek Kasireddy 2021-06-10 151 return; 47558b189c1d340 Dongwon Kim 2022-05-10 152 } 5c68ab9276aae47 Vivek Kasireddy 2021-06-10 153 virtio_gpu_array_add_obj(objs, vgfb->base.obj[0]); 5c68ab9276aae47 Vivek Kasireddy 2021-06-10 154 virtio_gpu_array_lock_resv(objs); 47558b189c1d340 Dongwon Kim 2022-05-10 155 } 47558b189c1d340 Dongwon Kim 2022-05-10 156 5c68ab9276aae47 Vivek Kasireddy 2021-06-10 157 virtio_gpu_cmd_resource_flush(vgdev, bo->hw_res_handle, x, y, 47558b189c1d340 Dongwon Kim 2022-05-10 158 width, height, objs, fence); 5c68ab9276aae47 Vivek Kasireddy 2021-06-10 159 virtio_gpu_notify(vgdev); 5c68ab9276aae47 Vivek Kasireddy 2021-06-10 160 47558b189c1d340 Dongwon Kim 2022-05-10 161 if (fence) { 47558b189c1d340 Dongwon Kim 2022-05-10 162 dma_fence_wait_timeout(&fence->f, true, 5c68ab9276aae47 Vivek Kasireddy 2021-06-10 163 msecs_to_jiffies(50)); 47558b189c1d340 Dongwon Kim 2022-05-10 164 dma_fence_put(&fence->f); 5c68ab9276aae47 Vivek Kasireddy 2021-06-10 165 } 5c68ab9276aae47 Vivek Kasireddy 2021-06-10 166 } 5c68ab9276aae47 Vivek Kasireddy 2021-06-10 167
Hi Dongwon,
url: https://github.com/intel-lab-lkp/linux/commits/Dongwon-Kim/drm-virtio-releas... base: git://anongit.freedesktop.org/drm/drm drm-next config: parisc-randconfig-m031-20220509 (https://download.01.org/0day-ci/archive/20220511/202205112132.FqtvzLWA-lkp@i...) compiler: hppa-linux-gcc (GCC) 11.3.0
If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com Reported-by: Dan Carpenter dan.carpenter@oracle.com
smatch warnings: drivers/gpu/drm/virtio/virtgpu_plane.c:158 virtio_gpu_resource_flush() error: uninitialized symbol 'objs'.
vim +/objs +158 drivers/gpu/drm/virtio/virtgpu_plane.c
5c68ab9276aae4 Vivek Kasireddy 2021-06-10 132 static void virtio_gpu_resource_flush(struct drm_plane *plane, 5c68ab9276aae4 Vivek Kasireddy 2021-06-10 133 uint32_t x, uint32_t y, 5c68ab9276aae4 Vivek Kasireddy 2021-06-10 134 uint32_t width, uint32_t height) 5c68ab9276aae4 Vivek Kasireddy 2021-06-10 135 { 5c68ab9276aae4 Vivek Kasireddy 2021-06-10 136 struct drm_device *dev = plane->dev; 5c68ab9276aae4 Vivek Kasireddy 2021-06-10 137 struct virtio_gpu_device *vgdev = dev->dev_private; 5c68ab9276aae4 Vivek Kasireddy 2021-06-10 138 struct virtio_gpu_framebuffer *vgfb; 5c68ab9276aae4 Vivek Kasireddy 2021-06-10 139 struct virtio_gpu_object *bo; 47558b189c1d34 Dongwon Kim 2022-05-10 140 struct virtio_gpu_object_array *objs; 47558b189c1d34 Dongwon Kim 2022-05-10 141 struct virtio_gpu_fence *fence; 5c68ab9276aae4 Vivek Kasireddy 2021-06-10 142 5c68ab9276aae4 Vivek Kasireddy 2021-06-10 143 vgfb = to_virtio_gpu_framebuffer(plane->state->fb); 5c68ab9276aae4 Vivek Kasireddy 2021-06-10 144 bo = gem_to_virtio_gpu_obj(vgfb->base.obj[0]); 5c7787de5069b5 Dongwon Kim 2022-05-10 145 fence = virtio_gpu_fence_alloc(vgdev, vgdev->fence_drv.context, 0); 47558b189c1d34 Dongwon Kim 2022-05-10 146 47558b189c1d34 Dongwon Kim 2022-05-10 147 if (fence) { 5c68ab9276aae4 Vivek Kasireddy 2021-06-10 148 objs = virtio_gpu_array_alloc(1); 47558b189c1d34 Dongwon Kim 2022-05-10 149 if (!objs) { 47558b189c1d34 Dongwon Kim 2022-05-10 150 kfree(fence); 5c68ab9276aae4 Vivek Kasireddy 2021-06-10 151 return; 47558b189c1d34 Dongwon Kim 2022-05-10 152 } 5c68ab9276aae4 Vivek Kasireddy 2021-06-10 153 virtio_gpu_array_add_obj(objs, vgfb->base.obj[0]); 5c68ab9276aae4 Vivek Kasireddy 2021-06-10 154 virtio_gpu_array_lock_resv(objs); 47558b189c1d34 Dongwon Kim 2022-05-10 155 }
"objs" not set on else path
47558b189c1d34 Dongwon Kim 2022-05-10 156 5c68ab9276aae4 Vivek Kasireddy 2021-06-10 157 virtio_gpu_cmd_resource_flush(vgdev, bo->hw_res_handle, x, y, 47558b189c1d34 Dongwon Kim 2022-05-10 @158 width, height, objs, fence); 5c68ab9276aae4 Vivek Kasireddy 2021-06-10 159 virtio_gpu_notify(vgdev); 5c68ab9276aae4 Vivek Kasireddy 2021-06-10 160 47558b189c1d34 Dongwon Kim 2022-05-10 161 if (fence) { 47558b189c1d34 Dongwon Kim 2022-05-10 162 dma_fence_wait_timeout(&fence->f, true, 5c68ab9276aae4 Vivek Kasireddy 2021-06-10 163 msecs_to_jiffies(50)); 47558b189c1d34 Dongwon Kim 2022-05-10 164 dma_fence_put(&fence->f); 5c68ab9276aae4 Vivek Kasireddy 2021-06-10 165 } 5c68ab9276aae4 Vivek Kasireddy 2021-06-10 166 }
dri-devel@lists.freedesktop.org
-
Dan Carpenter -
Dongwon Kim -
kernel test robot