search for: prepare_fb

...e_valid _very_ much belongs to this category too, since there's mode_valid hooks also on other objects. But simple pipe helper condenses that down to one mode_valid hook (we could also put the mode_valid onto encoder, wouldn't change anything). > In the latter category are mode_valid(), prepare_fb(), cleanup_fb(), > enable_vblank(), and disable_vblank(). IMHO those functions should > receive a pointer to the original structure as their first argument. > This type provides the context in which the operations make sense. (Even > their documentation already refers to the original st...

...e_valid _very_ much belongs to this category too, since there's mode_valid hooks also on other objects. But simple pipe helper condenses that down to one mode_valid hook (we could also put the mode_valid onto encoder, wouldn't change anything). > In the latter category are mode_valid(), prepare_fb(), cleanup_fb(), > enable_vblank(), and disable_vblank(). IMHO those functions should > receive a pointer to the original structure as their first argument. > This type provides the context in which the operations make sense. (Even > their documentation already refers to the original st...

Passing the wrong type feels icky, everywhere else we use the pipe as the first parameter. Spotted while discussing patches with Thomas Zimmermann. Cc: Thomas Zimmermann <tzimmermann at suse.de> Cc: Noralf Tr?nnes <noralf at tronnes.org> Cc: Gerd Hoffmann <kraxel at redhat.com> Cc: Eric Anholt <eric at anholt.net> Cc: Emil Velikov <emil.velikov at collabora.com> Cc:

Passing the wrong type feels icky, everywhere else we use the pipe as the first parameter. Spotted while discussing patches with Thomas Zimmermann. Cc: Thomas Zimmermann <tzimmermann at suse.de> Cc: Noralf Tr?nnes <noralf at tronnes.org> Cc: Gerd Hoffmann <kraxel at redhat.com> Cc: Eric Anholt <eric at anholt.net> Cc: Emil Velikov <emil.velikov at collabora.com> Cc:

...ngs to this category too, since there's > mode_valid hooks also on other objects. But simple pipe helper > condenses that down to one mode_valid hook (we could also put the > mode_valid onto encoder, wouldn't change anything). > >> In the latter category are mode_valid(), prepare_fb(), cleanup_fb(), >> enable_vblank(), and disable_vblank(). IMHO those functions should >> receive a pointer to the original structure as their first argument. >> This type provides the context in which the operations make sense. (Even >> their documentation already refers to...

No need to do the reservation dance, we can just wait on the fence directly. Signed-off-by: Gerd Hoffmann <kraxel at redhat.com> --- drivers/gpu/drm/virtio/virtgpu_plane.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c index 024c2aa0c929..4b805bf466d3 100644 ---

No need to do the reservation dance, we can just wait on the fence directly. Signed-off-by: Gerd Hoffmann <kraxel at redhat.com> --- drivers/gpu/drm/virtio/virtgpu_plane.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c index 024c2aa0c929..4b805bf466d3 100644 ---

...re genuine to simple KMS, and functions that are merely forwarded from another structure (crtc, plane, etc). In the former category are enable(), disable(), check(), and update(). Those should probably receive a simple display pipe as their first argument. In the latter category are mode_valid(), prepare_fb(), cleanup_fb(), enable_vblank(), and disable_vblank(). IMHO those functions should receive a pointer to the original structure as their first argument. This type provides the context in which the operations make sense. (Even their documentation already refers to the original structure.) I admit t...

...d_state) { @@ -831,7 +817,6 @@ static const uint32_t qxl_cursor_plane_formats[] = { }; static const struct drm_plane_helper_funcs qxl_cursor_helper_funcs = { - .atomic_check = qxl_plane_atomic_check, .atomic_update = qxl_cursor_atomic_update, .atomic_disable = qxl_cursor_atomic_disable, .prepare_fb = qxl_plane_prepare_fb, @@ -956,28 +941,6 @@ static int qdev_crtc_init(struct drm_device *dev, int crtc_id) return r; } -static void qxl_enc_dpms(struct drm_encoder *encoder, int mode) -{ - DRM_DEBUG("\n"); -} - -static void qxl_enc_prepare(struct drm_encoder *encoder) -{ - DRM_DEBUG...

This fixes a kernel oops when loading the nouveau module with fb console enabled after the change: drm/nouveau: Remove field nvbo from struct nouveau_framebuffer state->fb may be NULL in nv50_wndw_prepare_fb(), so defer initializing nvbo from its obj[] array until after the NULL check. Signed-off-by: James Jones <jajones at nvidia.com> --- drivers/gpu/drm/nouveau/dispnv50/wndw.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c b/drive...

...w_setbase(bochs, - plane->state->crtc_x, - plane->state->crtc_y, + state->crtc_x, + state->crtc_y, bo->bo.offset); - bochs_hw_setformat(bochs, plane->state->fb->format); + bochs_hw_setformat(bochs, state->fb->format); } -static int bochs_plane_prepare_fb(struct drm_plane *plane, - struct drm_plane_state *new_state) +static void bochs_pipe_enable(struct drm_simple_display_pipe *pipe, + struct drm_crtc_state *crtc_state, + struct drm_plane_state *plane_state) +{ + struct bochs_device *bochs = pipe->crtc.dev->dev_private; + +...

Hi I'm surprised that prepare_fb is called with fb == NULL. But, OK Acked-by: Thomas Zimmermann <tzimmermann at suse.de> Thanks for the fix. Am 11.02.20 um 00:09 schrieb James Jones: > This fixes a kernel oops when loading the nouveau > module with fb console enabled after the change: > > drm/nouveau: Remov...

...d_state) { @@ -831,7 +817,6 @@ static const uint32_t qxl_cursor_plane_formats[] = { }; static const struct drm_plane_helper_funcs qxl_cursor_helper_funcs = { - .atomic_check = qxl_plane_atomic_check, .atomic_update = qxl_cursor_atomic_update, .atomic_disable = qxl_cursor_atomic_disable, .prepare_fb = qxl_plane_prepare_fb, @@ -956,28 +941,6 @@ static int qdev_crtc_init(struct drm_device *dev, int crtc_id) return r; } -static void qxl_enc_dpms(struct drm_encoder *encoder, int mode) -{ - DRM_DEBUG("\n"); -} - -static void qxl_enc_prepare(struct drm_encoder *encoder) -{ - DRM_DEBUG...

...u_object_unreserve(bo); > - virtio_gpu_object_wait(bo, false); > - } > + dma_fence_wait(&vgfb->fence->f, true); > + dma_fence_put(&vgfb->fence->f); > + vgfb->fence = NULL; Even nicer would be to add the fence using drm_atomic_set_fence_for_plane() in the prepare_fb hook. Assuming this isn't necessary for correctness (but then I kinda have questions about races and stuff). But this gets the job done too I think, so: Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch> > } > > if (plane->state->fb != old_state->fb) { >...

...ne->state->fb) + return; + bo = gem_to_bochs_bo(plane->state->fb->obj[0]); + bochs_hw_setbase(bochs, + plane->state->crtc_x, + plane->state->crtc_y, + bo->bo.offset); + bochs_hw_setformat(bochs, plane->state->fb->format); +} + +static int bochs_plane_prepare_fb(struct drm_plane *plane, + struct drm_plane_state *new_state) +{ + struct bochs_bo *bo; + + if (!new_state->fb) + return 0; + bo = gem_to_bochs_bo(new_state->fb->obj[0]); + return bochs_bo_pin(bo, TTM_PL_FLAG_VRAM, NULL); +} + +static void bochs_plane_cleanup_fb(struct drm_plane *plane...

...e.c index 05022ef..77415e5 100644 --- a/drivers/gpu/drm/virtio/virtgpu_plane.c +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c @@ -115,6 +115,41 @@ static void virtio_gpu_primary_plane_update(struct drm_plane *plane, plane->state->src_h >> 16); } +static int virtio_gpu_cursor_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...

...e.c index 05022ef..77415e5 100644 --- a/drivers/gpu/drm/virtio/virtgpu_plane.c +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c @@ -115,6 +115,41 @@ static void virtio_gpu_primary_plane_update(struct drm_plane *plane, plane->state->src_h >> 16); } +static int virtio_gpu_cursor_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...

...u8 *src; /* @@ -397,8 +401,8 @@ static void vbox_cursor_atomic_update(struct drm_plane *plane, vbox_crtc->cursor_enabled = true; - src = drm_gem_vram_vmap(gbo); - if (IS_ERR(src)) { + ret = drm_gem_vram_vmap(gbo, &map); + if (ret) { /* * BUG: we should have pinned the BO in prepare_fb(). */ @@ -406,6 +410,7 @@ static void vbox_cursor_atomic_update(struct drm_plane *plane, DRM_WARN("Could not map cursor bo, skipping update\n"); return; } + src = map.vaddr; /* * The mask must be calculated based on the alpha @@ -416,7 +421,7 @@ static void vbox_cursor_...

...u8 *src; /* @@ -397,8 +401,8 @@ static void vbox_cursor_atomic_update(struct drm_plane *plane, vbox_crtc->cursor_enabled = true; - src = drm_gem_vram_vmap(gbo); - if (IS_ERR(src)) { + ret = drm_gem_vram_vmap(gbo, &map); + if (ret) { /* * BUG: we should have pinned the BO in prepare_fb(). */ @@ -406,6 +410,7 @@ static void vbox_cursor_atomic_update(struct drm_plane *plane, DRM_WARN("Could not map cursor bo, skipping update\n"); return; } + src = map.vaddr; /* TODO: Use mapping abstraction properly */ /* * The mask must be calculated based on the alpha...

...box_cursor_atomic_update(struct drm_plane *plane, > > vbox_crtc->cursor_enabled = true; > > - src = drm_gem_vram_vmap(gbo); > - if (IS_ERR(src)) { > + ret = drm_gem_vram_vmap(gbo, &map); > + if (ret) { > /* > * BUG: we should have pinned the BO in prepare_fb(). > */ > @@ -406,6 +410,7 @@ static void vbox_cursor_atomic_update(struct drm_plane *plane, > DRM_WARN("Could not map cursor bo, skipping update\n"); > return; > } > + src = map.vaddr; /* TODO: Use mapping abstraction properly */ > > /* >...