On Fri, Jan 24, 2025 at 11:48 AM Ian Forbes <ian.forbes@xxxxxxxxxxxx> wrote:
>
> On Tue, Jan 14, 2025 at 10:50 PM Zack Rusin <zack.rusin@xxxxxxxxxxxx> wrote:
> >
> > +int vmw_cursor_plane_prepare_fb(struct drm_plane *plane,
> > + struct drm_plane_state *new_state)
> > +{
> > + struct drm_framebuffer *fb = new_state->fb;
> > + struct vmw_cursor_plane *vcp = vmw_plane_to_vcp(plane);
> > + struct vmw_plane_state *vps = vmw_plane_state_to_vps(new_state);
> > + struct vmw_plane_state *old_vps = vmw_plane_state_to_vps(plane->state);
> > + struct vmw_private *vmw = vmw_priv(plane->dev);
> > + struct vmw_bo *bo = NULL;
> > + struct vmw_surface *surface;
> > + int ret = 0;
> > +
> > + if (!vmw_user_object_is_null(&vps->uo)) {
> > + vmw_user_object_unmap(&vps->uo);
> > + vmw_user_object_unref(&vps->uo);
> > + }
> > +
> > + if (fb) {
> > + if (vmw_framebuffer_to_vfb(fb)->bo) {
> > + vps->uo.buffer = vmw_framebuffer_to_vfbd(fb)->buffer;
> > + vps->uo.surface = NULL;
> > + } else {
> > + memcpy(&vps->uo, &vmw_framebuffer_to_vfbs(fb)->uo, sizeof(vps->uo));
> > + }
> > + vmw_user_object_ref(&vps->uo);
> > + }
> > +
> > + vps->cursor.update_type = vmw_cursor_update_type(vmw, vps);
> > + switch (vps->cursor.update_type) {
> > + case VMW_CURSOR_UPDATE_LEGACY:
> > + surface = vmw_user_object_surface(&vps->uo);
> > + if (!surface || vps->cursor.legacy.id == surface->snooper.id)
> > + vps->cursor.update_type = VMW_CURSOR_UPDATE_NONE;
> > + break;
> > + case VMW_CURSOR_UPDATE_MOB: {
> > + bo = vmw_user_object_buffer(&vps->uo);
> > + if (bo) {
> > + struct ttm_operation_ctx ctx = { false, false };
> > +
> > + ret = ttm_bo_reserve(&bo->tbo, true, false, NULL);
> > + if (ret != 0)
> > + return -ENOMEM;
> > +
> > + ret = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
> > + if (ret != 0)
> > + return -ENOMEM;
> > +
> > + /*
> > + * vmw_bo_pin_reserved also validates, so to skip
> > + * the extra validation use ttm_bo_pin directly
> > + */
> > + if (!bo->tbo.pin_count)
> > + ttm_bo_pin(&bo->tbo);
> > +
> > + if (vmw_framebuffer_to_vfb(fb)->bo) {
> > + const u32 size = new_state->crtc_w *
> > + new_state->crtc_h *
> > + sizeof(u32);
> > +
> > + (void)vmw_bo_map_and_cache_size(bo, size);
> > + } else {
> > + vmw_bo_map_and_cache(bo);
> > + }
> > + ttm_bo_unreserve(&bo->tbo);
> > + }
> > + if (!vmw_user_object_is_null(&vps->uo)) {
> > + if (!vmw_cursor_plane_changed(vps, old_vps) &&
> > + !vmw_cursor_buffer_changed(vps, old_vps)) {
> > + vps->cursor.update_type =
> > + VMW_CURSOR_UPDATE_NONE;
> > + } else {
> > + vmw_cursor_mob_get(vcp, vps);
> > + vmw_cursor_mob_map(vps);
> > + }
> > + }
> > + }
> > + break;
> > + case VMW_CURSOR_UPDATE_NONE:
> > + /* do nothing */
> > + break;
> > + }
> > +
> > + return 0;
> > +}
>
> Is there any way we can determine the update type at KMS init time and
> create a different drm_plane_helper_funcs vtable for each type?
> Essentially splitting this function, atomic_check, etc. into 4
> variants and removing all the switches.
No, unfortunately we can't. The cursor snooper depends on the
framebuffer surface that's been assigned to the plane and not on
capabilities of the device that we'd have access to at the init time,
i.e. it's the client that dictates it and at init time we don't know
what clients would use this.
Sadly cursor snooper is only relevant on very old xservers, so the
question would be "can we get rid of the cursor snooper" and, also,
unfortunately because we can't break old userspace the answer is no.
z
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
