nged, 1031 insertions(+), 917 deletions(-)
> create mode 100644 drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.c
> create mode 100644 drivers/gpu/drm/vmwgfx/vmwgfx_cursor_plane.h
>
> --
> 2.45.2
>
LGTM to the series.
Reviewed-by: Martin Krastev
Regards,
Martin
smime.p7s
Description: S/MIME Cryptographic Signature
On Wed, Jan 15, 2025 at 6:50 AM Zack Rusin wrote:
>
> Refactor cursor handling to make the code maintainable again. Over the
> last 12 years the svga device improved support for virtualized cursors
> and at the same time the drm interfaces evolved quite a bit from
> pre-atomic to current atomic on
dex 43b5439ec9f76..07749f0a5f294 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.h
> @@ -83,7 +83,6 @@ struct vmw_bo {
>
> struct ttm_placement placement;
> struct ttm_place places[5];
> - struct ttm_place busy_places
quot;drm/vmwgfx: Fix handling of dumb buffers")
> Cc: Broadcom internal kernel review list
>
> Cc: dri-devel@lists.freedesktop.org
> Cc: # v6.9+
> Cc: Maaz Mombasawala
> Cc: Martin Krastev
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 29 ---
Still LGTM.
Reviewed-by: Martin Krastev
Regards,
Martin
On Fri, Aug 16, 2024 at 9:33 PM Zack Rusin wrote:
>
> This is the same series I've sent out earlier but with one extra patch,
> that fixes the dumb buffer coherency on low mem systems.
>
> The second patch has also
LGTM.
Reviewed-by: Martin Krastev
Regards,
Martin
On Fri, Aug 9, 2024 at 9:38 PM Ian Forbes wrote:
>
> Handle unlikely ENOMEN condition and other errors in
> vmw_stdu_connector_atomic_check.
>
> Signed-off-by: Ian Forbes
> Reported-by: Dan Carpenter
> Fixes: 75c3
2 unit_y2;
> };
>
> -#define VMWGFX_NUM_DISPLAY_UNITS 8
> -
> -
> #define vmw_framebuffer_to_vfb(x) \
> container_of(x, struct vmw_framebuffer, base)
> #define vmw_framebuffer_to_vfbs(x) \
> --
> 2.34.1
>
LGTM.
Reviewed-by: Martin Krastev
Regards,
Martin
urrently active maps. Will differ from the
> + * cpu_writers because it includes kernel maps.
> * @cpu_writers: Number of synccpu write grabs. Protected by reservation when
> * increased. May be decreased without reservation.
> * @dx_query_ctx: DX context if this buffer object is used as a DX query MOB
> @@ -90,6 +92,7 @@ struct vmw_bo {
> u32 res_prios[TTM_MAX_BO_PRIORITY];
> struct xarray detached_resources;
>
> + atomic_t map_count;
> atomic_t cpu_writers;
> /* Not ref-counted. Protected by binding_mutex */
> struct vmw_resource *dx_query_ctx;
> --
> 2.43.0
>
LGTM.
Reviewed-by: Martin Krastev
Regards,
Martin
fset, dst_pitch,
> @@ -1143,7 +1143,7 @@ vmw_stdu_bo_populate_update_cpu(struct
> vmw_du_update_plane *update, void *cmd,
> struct vmw_diff_cpy diff = VMW_CPU_BLIT_DIFF_INITIALIZER(0);
> struct vmw_stdu_update_gb_image *cmd_img = cmd;
> struct vmw_stdu_update *cmd_update;
> - struct ttm_buffer_object *src_bo, *dst_bo;
> + struct vmw_bo *src_bo, *dst_bo;
> u32 src_offset, dst_offset;
> s32 src_pitch, dst_pitch;
> s32 width, height;
> @@ -1157,11 +1157,11 @@ vmw_stdu_bo_populate_update_cpu(struct
> vmw_du_update_plane *update, void *cmd,
>
> diff.cpp = stdu->cpp;
>
> - dst_bo = &stdu->display_srf->res.guest_memory_bo->tbo;
> + dst_bo = stdu->display_srf->res.guest_memory_bo;
> dst_pitch = stdu->display_srf->metadata.base_size.width * stdu->cpp;
> dst_offset = bb->y1 * dst_pitch + bb->x1 * stdu->cpp;
>
> - src_bo = &vfbbo->buffer->tbo;
> + src_bo = vfbbo->buffer;
> src_pitch = update->vfb->base.pitches[0];
> src_offset = bo_update->fb_top * src_pitch + bo_update->fb_left *
> stdu->cpp;
> --
> 2.43.0
>
LGTM, just with those two remarks.
Reviewed-by: Martin Krastev
Regards,
Martin
ence, *next_fence;
>
> if (likely(!fman->seqno_valid))
> return false;
> @@ -413,7 +410,7 @@ static bool vmw_fence_goal_new_locked(struct
> vmw_fence_manager *fman,
> return false;
>
> fman->seqno_valid = false;
> - list_for_each_entry(fence, &fman->fence_list, head) {
> + list_for_each_entry_safe(fence, next_fence, &fman->fence_list, head) {
> if (!list_empty(&fence->seq_passed_actions)) {
> fman->seqno_valid = true;
> vmw_fence_goal_write(fman->dev_priv,
> --
> 2.43.0
>
LGTM
Reviewed-by: Martin Krastev
Regards,
Martin
gpu/drm/vmwgfx/vmwgfx_resource.c | 27 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 33 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 145 +++
> drivers/gpu/drm/vmwgfx/vmwgfx_surface.c| 277 -
> drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c | 40 +-
> 15 files changed, 788 insertions(+), 530 deletions(-)
>
> --
> 2.40.1
>
To the series:
Reviewed-by: Martin Krastev
Regards,
Martin
.pin = vmw_gem_object_pin,
> .unpin = vmw_gem_object_unpin,
> .get_sg_table = vmw_gem_object_get_sg_table,
> - .vmap = drm_gem_ttm_vmap,
> - .vunmap = drm_gem_ttm_vunmap,
> - .mmap = drm_gem_ttm_mmap,
> + .vmap = vmw_gem_vmap,
> + .vunmap = vmw_gem_vunmap,
> + .mmap = vmw_gem_mmap,
> .vm_ops = &vmw_vm_ops,
> };
>
> --
> 2.40.1
>
LGTM!
Reviewed-by: Martin Krastev
Regards,
Martin
ack Rusin
> Fixes: 7b0062036c3b ("drm/vmwgfx: Implement virtual crc generation")
> Cc: Zack Rusin
> Cc: Martin Krastev
> Cc: Broadcom internal kernel review list
>
> Cc: dri-devel@lists.freedesktop.org
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c | 37 ++
> + fence->base.ops = &vmw_fence_ops_removed;
> list_del_init(&fence->head);
> dma_fence_signal_locked(&fence->base);
> INIT_LIST_HEAD(&action_list);
> @@ -662,6 +675,7 @@ void vmw_fence_fifo_down(struct vmw_fence_manager *fman)
> VMW_FENCE_WAIT_TIMEOUT);
>
> if (unlikely(ret != 0)) {
> + fence->base.ops = &vmw_fence_ops_removed;
> list_del_init(&fence->head);
> dma_fence_signal(&fence->base);
> INIT_LIST_HEAD(&action_list);
> --
> 2.40.1
>
Neat fix!
Reviewed-by: Martin Krastev
Regards,
Martin
if (new_crtc_state->enable &&
> + !new_crtc_state->active &&
> + !new_crtc_state->mode_changed)
> + return;
> +
> ret = vmw_stdu_destroy_st(dev_priv, stdu);
> if (ret)
> DRM_ERROR("Failed to destroy Screen Target\n");
> --
> 2.34.1
>
LGTM!
Reviewed-by: Martin Krastev
Regards,
Martin
LGTM!
Reviewed-by: Martin Krastev
Regards,
Martin
On Thu, Apr 25, 2024 at 10:27 PM Zack Rusin wrote:
>
> Correctly set the length of the drm_event to the size of the structure
> that's actually used.
>
> The length of the drm_event was set to the parent structure
Good catch!
Reviewed-by: Martin Krastev
Regards,
Martin
On Thu, Apr 25, 2024 at 11:07 PM Ian Forbes wrote:
>
> Legacy DU was broken by the referenced fixes commit because the placement
> and the busy_placement no longer pointed to the same object. This was later
> fixed indirect
On Fri, Apr 12, 2024 at 5:36 PM Martin Krastev
wrote:
>
> On Fri, Apr 12, 2024 at 5:55 AM Zack Rusin wrote:
> >
> > vmwgfx didn't have support for vblank or crc generation which made it
> > impossible to use a large number of IGT tests to properly test DRM
&g
s.c | 632 +
> drivers/gpu/drm/vmwgfx/vmwgfx_vkms.h | 75 +++
> 17 files changed, 965 insertions(+), 109 deletions(-)
> create mode 100644 drivers/gpu/drm/vmwgfx/vmwgfx_vkms.c
> create mode 100644 drivers/gpu/drm/vmwgfx/vmwgfx_vkms.h
>
> --
> 2.40.1
>
LGTM to the series.
Signed-off-by: Martin Krastev
Regards,
Martin
Great to have this!
Reviewed-by: Martin Krastev
Regards,
Martin
On Wed, Apr 3, 2024 at 2:28 AM Zack Rusin wrote:
>
> vmwgfx never supported prime import of external buffers. Furthermore the
> driver exposes two different objects to userspace: vmw_surface's and
> gem buffers
ral, but broken userspace might expect
> +* some actual rendering so give a clue as why it's blank.
> +*/
> + if (new_state->enable && !has_primary)
> + drm_dbg_driver(&vmw->drm,
> + "CRTC without a primary plane will be
> blank.\n");
>
>
> if (new_state->connector_mask != connector_mask &&
> --
> 2.40.1
>
LGTM!
Reviewed-by: Martin Krastev
Regards,
Martin
On Wed, Apr 3, 2024 at 2:28 AM Zack Rusin wrote:
>
> crc checksums are used to validate the output. Normally they're part
> of the actual display hardware but on virtual stack there's nothing
> to automatically generate them.
>
> Implement crc generation for the vmwgfx stack. This works only on
>
orm_has(CC_ATTR_MEM_ENCRYPT))
> dev_priv->map_mode = vmw_dma_alloc_coherent;
> else if (vmw_restrict_iommu)
> dev_priv->map_mode = vmw_dma_map_bind;
> --
> 2.40.1
LGTM!
Reviewed-by: Martin Krastev
Regards,
Martin
m)) {
> - const int ret = ttm_bo_reserve(&vps->bo->tbo, true, false,
> NULL);
> -
> - if (likely(ret == 0)) {
> - ttm_bo_kunmap(&vps->bo->map);
> - ttm_bo_unreserve(&vps->bo->tbo);
> - }
> - }
> -
> vmw_du_cursor_plane_unmap_cm(vps);
> vmw_du_put_cursor_mob(vcp, vps);
>
> --
> 2.40.1
>
LGTM!
Reviewed-by: Martin Krastev
Regards,
Martin
on to discover it.
> Niels De Graef who reported it and helped to track down the poc.
>
> Fixes: 9c079b8ce8bf ("drm/vmwgfx: Adapt execbuf to the new validation api")
> Cc: # v4.20+
> Reported-by: Niels De Graef
> Signed-off-by: Zack Rusin
> Cc: Martin Krastev
&g
Great catch! That was a nasty one.
Reviewed-by: Martin Krastev
Regards,
Martin
On Sun, Dec 24, 2023 at 7:29 AM Zack Rusin wrote:
> Switch to a new plane state requires unreferencing of all held surfaces.
> In the work required for mob cursors the mapped surfaces started being
> c
From: Martin Krastev
LGTM
Reviewed-by: Martin Krastev
Regards,
Martin
On 28.09.23 г. 7:13 ч., Zack Rusin wrote:
From: Zack Rusin
Surfaces can be backed (i.e. stored in) memory objects (mob's) which
are created and managed by the userspace as GEM buffers. Surfaces
grab only
From: Martin Krastev
LGTM!
Reviewed-by: Martin Krastev
Regards,
Martin
On 18 Aug 2023 04:13:14, Zack Rusin wrote:
>From: Zack Rusin
>
>vmw_bo_unreference sets the input buffer to null on exit, resulting in
>null ptr deref's on the subsequent drm gem put calls.
>
From: Martin Krastev
Looks good.
Reviewed-by: Martin Krastev
Regards,
Martin
On 27.06.23 г. 6:58 ч., Zack Rusin wrote:
From: Zack Rusin
Atomic modesetting got support for mouse hotspots via the hotspot
properties. Port the legacy kms hotspot handling to the new properties
on cursor
From: Martin Krastev
Looks good!
Reviewed-by: Martin Krastev
Regards,
Martin
On 16.06.23 г. 22:09 ч., Zack Rusin wrote:
From: Zack Rusin
For multiple commands the driver was not correctly validating the shader
stages resulting in possible kernel oopses. The validation code was only
From: Martin Krastev
LGTM!
Reviewed-by: Martin Krastev
Regards,
Martin
On 5.04.23 г. 7:56 ч., Zack Rusin wrote:
From: Zack Rusin
Many drivers (in particular all of the virtualized drivers) do not
implement vblank handling. Assuming vblank is always present
leads to crashes.
Fix the
From: Martin Krastev
LGTM!
Reviewed-by: Martin Krastev
Regards,
Martin
On 21.03.23 г. 4:09 ч., Zack Rusin wrote:
From: Zack Rusin
virtualbox implemented an incomplete version of the svga device which
they decided to drop soon after the initial release. The device was
always broken in
From: Martin Krastev
We reviewers botched that one.
Reviewed-by: Martin Krastev
Regards,
Martin
On 14.03.23 г. 23:14 ч., Zack Rusin wrote:
From: Zack Rusin
The src/dst_pitch got mixed up during the rework of the function, make
sure the offset's refer to the correct one.
Spott
From: Martin Krastev
Much nicer now.
Reviewed-by: Martin Krastev
Regards,
Martin
On 8.02.23 г. 20:00 ч., Zack Rusin wrote:
From: Zack Rusin
ttm_bo_init_reserved on failure puts the buffer object back which
causes it to be deleted, but kfree was still being called on the same
buffer in
From: Martin Krastev
LGTM
Reviewed-by: Martin Krastev
Regards,
Martin
On 9.02.23 г. 14:37 ч., Zack Rusin wrote:
From: Zack Rusin
v2: Update the commit message to include note describing why the second
usag of vmw_gem_object_create_with_handle in vmwgfx_surface.c wasn't
changed
From: Martin Krastev
Looks good to me.
Reviewed-by: Martin Krastev
Regards,
Martin
On 8.02.23 г. 23:53 ч., Zack Rusin wrote:
From: Zack Rusin
It is possible for userspace to predict the next buffer handle and
to destroy the buffer while it's still used by the kernel. Delay
droppin
From: Martin Krastev
LGTM!
Reviewed-by: Martin Krastev
Regards,
Martin
On 31.01.23 г. 5:35 ч., Zack Rusin wrote:
From: Zack Rusin
Various bits of the driver used raw ttm_buffer_object instead of the
driver specific vmw_bo object. All those places used to duplicate
the mapped bo caching
From: Martin Krastev
LGTM
Reviewed-by: Martin Krastev
Regards,
Martin
On 31.01.23 г. 5:35 ч., Zack Rusin wrote:
From: Zack Rusin
Rename dummy to is_iomem because that's what it is even if we're not
activelly using it. Makes the code easier to read.
Signed-off-by:
From: Martin Krastev
LGTM
Reviewed-by: Martin Krastev
Regards,
Martin
On 31.01.23 г. 5:35 ч., Zack Rusin wrote:
From: Zack Rusin
Problem with explicit placement selection in vmwgfx is that by the time
the buffer object needs to be validated the information about which
placement was
From: Martin Krastev
LGTM
Reviewed-by: Martin Krastev
Regards,
Martin
On 31.01.23 г. 5:35 ч., Zack Rusin wrote:
From: Zack Rusin
Before vmwgfx supported gem it needed to implement the entire mmap logic
explicitly. With GEM support that's not needed and the generic code
can be us
From: Martin Krastev
Ah, good!
Reviewed-by: Martin Krastev
Regards,
Martin
On 26.01.23 г. 19:38 ч., Zack Rusin wrote:
From: Zack Rusin
Base mapped count is useless because the ttm unmap functions handle
null maps just fine so completely remove all the code related to it.
Rename dummy
From: Martin Krastev
LGTM!
Reviewed-by: Martin Krastev
Regards,
Martin
On 26.01.23 г. 19:38 ч., Zack Rusin wrote:
From: Zack Rusin
Only the legacy display unit requires pinning of the fb memory in vram.
Both the screen objects and screen targets can present from any buffer.
That makes
From: Martin Krastev
LGTM!
Reviewed-by: Martin Krastev
Regards,
Martin
On 26.01.23 г. 19:38 ч., Zack Rusin wrote:
From: Zack Rusin
The rest of the drivers which are using ttm have mostly standardized on
driver_prefix_bo as the name for subclasses of the TTM buffer object.
Make vmwgfx
From: Martin Krastev
LGTM!
Reviewed-by: Martin Krastev
Regards,
Martin
On 26.01.23 г. 19:38 ч., Zack Rusin wrote:
From: Zack Rusin
Remove the explicit bo_free parameter which was switching between
vmw_bo_bo_free and vmw_gem_destroy which had exactly the same
implementation.
It makes
From: Martin Krastev
LGTM!
Reviewed-by: Martin Krastev
Regards,
Martin
On 26.01.23 г. 19:38 ч., Zack Rusin wrote:
From: Zack Rusin
Before vmwgfx supported gem it needed to implement the entire mmap logic
explicitly. With GEM support that's not needed and the generic code
can be us
From: Martin Krastev
LGTM!
Reviewed-by: Martin Krastev
Regards,
Martin
On 11.01.23 г. 19:50 ч., Zack Rusin wrote:
From: Zack Rusin
The branch is explicitly taken if ttm == NULL which means that to avoid
a null pointer reference the ttm object can not be used inside. Switch
back to
From: Martin Krastev
Looks good!
Reviewed-by: Martin Krastev
Regards,
Martin
On 7.12.22 г. 19:29 ч., Zack Rusin wrote:
From: Zack Rusin
User resource lookups used rcu to avoid two extra atomics. Unfortunately
the rcu paths were buggy and it was easy to make the driver crash by
From: Martin Krastev
LGTM!
Reviewed-by: Martin Krastev
Regards,
Martin
On 1.12.22 г. 19:53 ч., Zack Rusin wrote:
From: Zack Rusin
When SEV is enabled gmr's and mob's are explicitly disabled because
the encrypted system memory can not be used by the hypervisor.
The
From: Martin Krastev
Looks great!
Reviewed-by: Martin Krastev
Regards,
Martin
On 9.11.22 г. 17:37 ч., Dawei Li wrote:
pin_user_pages() is unsafe without protection of mmap_lock,
fix it by calling pin_user_pages_fast().
Fixes: 7a7a933edd6c ("drm/vmwgfx: Introduce VMware mks-guest-
From: Martin Krastev
Thanks for the catch. LGTM, with the following remarks:
1) Original design used erroneously pin_user_pages() in place of
pin_user_pages_fast(); you could just substitute pin_user_pages for
pin_user_pages_fast and call it a day, Please, consider that option
after
From: Martin Krastev
On 21.10.22 г. 6:44 ч., Zack Rusin wrote:
From: Zack Rusin
Fixes a warning about extra docs about a function argument that has been
removed a while back:
drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c:3888: warning: Excess function
parameter 'sync_file' desc
Thank you for the catch!
Reviewed-by: Martin Krastev
Regards,
Martin
On 16.09.22 г. 23:47 ч., Rafael Mendonca wrote:
If the copy of the description string from userspace fails, then the page
for the instance descriptor doesn't get freed before returning -EFAULT,
which leads to a me
From: Martin Krastev
On 12.07.22 г. 6:32 ч., Zack Rusin wrote:
From: Zack Rusin
Atomic modesetting got support for mouse hotspots via the hotspot
properties. Port the legacy kms hotspot handling to the new properties
on cursor planes.
Signed-off-by: Zack Rusin
Cc: Martin Krastev
Cc: Maaz
From: Martin Krastev
On 2.06.22 г. 18:42 ч., Zack Rusin wrote:
From: Zack Rusin
Atomic modesetting got support for mouse hotspots via the hotspot
properties. Drivers need to create those properties on cursor planes
which require the mouse hotspot coordinates.
Add the code creating hotspot
From: Martin Krastev
From: Zack Rusin
Date: Wed, 20 Apr 2022 at 07:03
Subject: [PATCH v2] drm/vmwgfx: Fix gem refcounting and memory evictions
To:
Cc: ,
From: Zack Rusin
v2: Add the last part of the ref count fix which was spotted by
Philipp Sieweck where the ref count of cpu writers
gt; - if (!(flags & drm_vmw_synccpu_allow_cs)) {
> - atomic_dec(&vmw_bo->cpu_writers);
> + if (!ret) {
> + if (!(flags & drm_vmw_synccpu_allow_cs)) {
> + atomic_dec(&vmw_bo->cpu_writers);
> +
v(struct vmw_private *dev_priv,
> if (!has_sm5_context(dev_priv))
> return -EINVAL;
>
> - if (num_uav > SVGA3D_MAX_UAVIEWS) {
> + if (num_uav > vmw_max_num_uavs(dev_priv)) {
> VMW_DEBUG_USER("Invalid UAV binding.\n");
> return -EINVAL;
> }
> @@ -2948,7 +2948,7 @@ static int vmw_cmd_set_cs_uav(struct vmw_private
> *dev_priv,
> if (!has_sm5_context(dev_priv))
> return -EINVAL;
>
> - if (num_uav > SVGA3D_MAX_UAVIEWS) {
> + if (num_uav > vmw_max_num_uavs(dev_priv)) {
> VMW_DEBUG_USER("Invalid UAV binding.\n");
> return -EINVAL;
> }
Reviewed-by: Martin Krastev
a_overlay.h | 6 +++---
> drivers/gpu/drm/vmwgfx/device_include/svga_reg.h | 14 --
> 9 files changed, 41 insertions(+), 34 deletions(-)
Reviewed-by: Martin Krastev
t; if (vmw_tt->mapped)
> @@ -321,16 +319,6 @@ static int vmw_ttm_map_dma(struct vmw_ttm_tt *vmw_tt)
> break;
> }
>
> - old = ~((dma_addr_t) 0);
> - vmw_tt->vsgt.num_regions = 0;
> - for (vmw_piter_start(&iter, vsgt, 0); vmw_piter_next(&iter);) {
> - dma_addr_t cur = vmw_piter_dma_addr(&iter);
> -
> - if (cur != old + PAGE_SIZE)
> - vmw_tt->vsgt.num_regions++;
> - old = cur;
> - }
> -
> vmw_tt->mapped = true;
> return 0;
>
Reviewed-by: Martin Krastev
quot;20210722"
> +#define VMWGFX_DRIVER_DATE "20211206"
> #define VMWGFX_DRIVER_MAJOR 2
> -#define VMWGFX_DRIVER_MINOR 19
> +#define VMWGFX_DRIVER_MINOR 20
> #define VMWGFX_DRIVER_PATCHLEVEL 0
> #define VMWGFX_FIFO_STATIC_SIZE (1024*1024)
> #define VMWGFX_MAX_RELOCATIONS 2048
Reviewed-by: Martin Krastev
60 matches
Mail list logo
