So ... for Vulkan, the API requires allocating VA before declaring
what goes into that VA (e.g. an image or data). I believe our plan for
that was to move all this into userspace, which would allocate VA and
then just tell the kernel "make VA range X have memtype Y". At that
point, nouveau_bo would be left mainly for compat as well as for
things like framebuffer backing.
James, in what situation would the modifier be different than the bo's memtype?
-ilia
On Thu, Feb 6, 2020 at 10:43 AM James Jones <jajones@xxxxxxxxxx> wrote:
>
> The format modifiers, when present, override the equivalent field in the
> BO. Right now, that's probably not particularly useful. However, as
> nouveau interfaces evolve towards the HW capabilities and add support
> for newer graphics APIs, saying an entire BO has a singular layout
> becomes less meaningful, so I suspect those fields will be effectively
> deprecated in favor of the modifier-defined and, for non-FB operations,
> strictly userspace defined layout. Doing so will be much easier if the
> modifier support is already in place for applications to start making
> use of.
>
> Thanks,
> -James
>
> On 2/6/20 7:28 AM, Roy Spliet wrote:
> > (Re-sending to list rather than just to James)
> >
> > Is this format modifier information not stored, or otherwise worth
> > storing, directly in the nouveau_bo object associated with the
> > framebuffer? I'm not particularly knowledgeable on the topic, but there
> > already seem to exist some fields with very similar names in nouveau_bo.
> > Best,
> >
> > Roy
> >
> > On 06/02/2020 15:17, James Jones wrote:
> >> Note I'm adding some fields to nouveau_framebuffer in the series
> >> "drm/nouveau: Support NVIDIA format modifiers." I sent out v3 of that
> >> yesterday. It would probably still be possible to avoid them by
> >> re-extracting the relevant data from the format modifier on the fly
> >> when needed, but it is simpler and likely less error-prone with the
> >> wrapper struct.
> >>
> >> Thanks,
> >> -James
> >>
> >> On 2/6/20 2:19 AM, Thomas Zimmermann wrote:
> >>> After its cleanup, struct nouveau_framebuffer is only a wrapper around
> >>> struct drm_framebuffer. Use the latter directly.
> >>>
> >>> Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
> >>> ---
> >>> drivers/gpu/drm/nouveau/dispnv50/wndw.c | 26 +++++++++++------------
> >>> drivers/gpu/drm/nouveau/nouveau_display.c | 14 ++++++------
> >>> drivers/gpu/drm/nouveau/nouveau_display.h | 12 +----------
> >>> drivers/gpu/drm/nouveau/nouveau_fbcon.c | 14 ++++++------
> >>> 4 files changed, 28 insertions(+), 38 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
> >>> b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
> >>> index ba1399965a1c..4a67a656e007 100644
> >>> --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
> >>> +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
> >>> @@ -40,11 +40,11 @@ nv50_wndw_ctxdma_del(struct nv50_wndw_ctxdma
> >>> *ctxdma)
> >>> }
> >>> static struct nv50_wndw_ctxdma *
> >>> -nv50_wndw_ctxdma_new(struct nv50_wndw *wndw, struct
> >>> nouveau_framebuffer *fb)
> >>> +nv50_wndw_ctxdma_new(struct nv50_wndw *wndw, struct drm_framebuffer
> >>> *fb)
> >>> {
> >>> - struct nouveau_drm *drm = nouveau_drm(fb->base.dev);
> >>> + struct nouveau_drm *drm = nouveau_drm(fb->dev);
> >>> struct nv50_wndw_ctxdma *ctxdma;
> >>> - struct nouveau_bo *nvbo = nouveau_gem_object(fb->base.obj[0]);
> >>> + struct nouveau_bo *nvbo = nouveau_gem_object(fb->obj[0]);
> >>> const u8 kind = nvbo->kind;
> >>> const u32 handle = 0xfb000000 | kind;
> >>> struct {
> >>> @@ -236,16 +236,16 @@ nv50_wndw_atomic_check_acquire(struct nv50_wndw
> >>> *wndw, bool modeset,
> >>> struct nv50_wndw_atom *asyw,
> >>> struct nv50_head_atom *asyh)
> >>> {
> >>> - struct nouveau_framebuffer *fb =
> >>> nouveau_framebuffer(asyw->state.fb);
> >>> + struct drm_framebuffer *fb = asyw->state.fb;
> >>> struct nouveau_drm *drm = nouveau_drm(wndw->plane.dev);
> >>> - struct nouveau_bo *nvbo = nouveau_gem_object(fb->base.obj[0]);
> >>> + struct nouveau_bo *nvbo = nouveau_gem_object(fb->obj[0]);
> >>> int ret;
> >>> NV_ATOMIC(drm, "%s acquire\n", wndw->plane.name);
> >>> - if (asyw->state.fb != armw->state.fb || !armw->visible ||
> >>> modeset) {
> >>> - asyw->image.w = fb->base.width;
> >>> - asyw->image.h = fb->base.height;
> >>> + if (fb != armw->state.fb || !armw->visible || modeset) {
> >>> + asyw->image.w = fb->width;
> >>> + asyw->image.h = fb->height;
> >>> asyw->image.kind = nvbo->kind;
> >>> ret = nv50_wndw_atomic_check_acquire_rgb(asyw);
> >>> @@ -261,13 +261,13 @@ nv50_wndw_atomic_check_acquire(struct nv50_wndw
> >>> *wndw, bool modeset,
> >>> asyw->image.blockh = nvbo->mode >> 4;
> >>> else
> >>> asyw->image.blockh = nvbo->mode;
> >>> - asyw->image.blocks[0] = fb->base.pitches[0] / 64;
> >>> + asyw->image.blocks[0] = fb->pitches[0] / 64;
> >>> asyw->image.pitch[0] = 0;
> >>> } else {
> >>> asyw->image.layout = 1;
> >>> asyw->image.blockh = 0;
> >>> asyw->image.blocks[0] = 0;
> >>> - asyw->image.pitch[0] = fb->base.pitches[0];
> >>> + asyw->image.pitch[0] = fb->pitches[0];
> >>> }
> >>> if (!asyh->state.async_flip)
> >>> @@ -486,16 +486,16 @@ nv50_wndw_cleanup_fb(struct drm_plane *plane,
> >>> struct drm_plane_state *old_state)
> >>> static int
> >>> nv50_wndw_prepare_fb(struct drm_plane *plane, struct
> >>> drm_plane_state *state)
> >>> {
> >>> - struct nouveau_framebuffer *fb = nouveau_framebuffer(state->fb);
> >>> + struct drm_framebuffer *fb = state->fb;
> >>> struct nouveau_drm *drm = nouveau_drm(plane->dev);
> >>> struct nv50_wndw *wndw = nv50_wndw(plane);
> >>> struct nv50_wndw_atom *asyw = nv50_wndw_atom(state);
> >>> - struct nouveau_bo *nvbo = nouveau_gem_object(state->fb->obj[0]);
> >>> + struct nouveau_bo *nvbo = nouveau_gem_object(fb->obj[0]);
> >>> struct nv50_head_atom *asyh;
> >>> struct nv50_wndw_ctxdma *ctxdma;
> >>> int ret;
> >>> - NV_ATOMIC(drm, "%s prepare: %p\n", plane->name, state->fb);
> >>> + NV_ATOMIC(drm, "%s prepare: %p\n", plane->name, fb);
> >>> if (!asyw->state.fb)
> >>> return 0;
> >>> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c
> >>> b/drivers/gpu/drm/nouveau/nouveau_display.c
> >>> index bbbff55eb5d5..94f7fd48e1cf 100644
> >>> --- a/drivers/gpu/drm/nouveau/nouveau_display.c
> >>> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
> >>> @@ -207,10 +207,10 @@ int
> >>> nouveau_framebuffer_new(struct drm_device *dev,
> >>> const struct drm_mode_fb_cmd2 *mode_cmd,
> >>> struct drm_gem_object *gem,
> >>> - struct nouveau_framebuffer **pfb)
> >>> + struct drm_framebuffer **pfb)
> >>> {
> >>> struct nouveau_drm *drm = nouveau_drm(dev);
> >>> - struct nouveau_framebuffer *fb;
> >>> + struct drm_framebuffer *fb;
> >>> int ret;
> >>> /* YUV overlays have special requirements pre-NV50 */
> >>> @@ -236,10 +236,10 @@ nouveau_framebuffer_new(struct drm_device *dev,
> >>> if (!(fb = *pfb = kzalloc(sizeof(*fb), GFP_KERNEL)))
> >>> return -ENOMEM;
> >>> - drm_helper_mode_fill_fb_struct(dev, &fb->base, mode_cmd);
> >>> - fb->base.obj[0] = gem;
> >>> + drm_helper_mode_fill_fb_struct(dev, fb, mode_cmd);
> >>> + fb->obj[0] = gem;
> >>> - ret = drm_framebuffer_init(dev, &fb->base,
> >>> &nouveau_framebuffer_funcs);
> >>> + ret = drm_framebuffer_init(dev, fb, &nouveau_framebuffer_funcs);
> >>> if (ret)
> >>> kfree(fb);
> >>> return ret;
> >>> @@ -250,7 +250,7 @@ nouveau_user_framebuffer_create(struct drm_device
> >>> *dev,
> >>> struct drm_file *file_priv,
> >>> const struct drm_mode_fb_cmd2 *mode_cmd)
> >>> {
> >>> - struct nouveau_framebuffer *fb;
> >>> + struct drm_framebuffer *fb;
> >>> struct drm_gem_object *gem;
> >>> int ret;
> >>> @@ -260,7 +260,7 @@ nouveau_user_framebuffer_create(struct drm_device
> >>> *dev,
> >>> ret = nouveau_framebuffer_new(dev, mode_cmd, gem, &fb);
> >>> if (ret == 0)
> >>> - return &fb->base;
> >>> + return fb;
> >>> drm_gem_object_put_unlocked(gem);
> >>> return ERR_PTR(ret);
> >>> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h
> >>> b/drivers/gpu/drm/nouveau/nouveau_display.h
> >>> index 56c1dec8fc28..082bb067d575 100644
> >>> --- a/drivers/gpu/drm/nouveau/nouveau_display.h
> >>> +++ b/drivers/gpu/drm/nouveau/nouveau_display.h
> >>> @@ -8,21 +8,11 @@
> >>> #include <drm/drm_framebuffer.h>
> >>> -struct nouveau_framebuffer {
> >>> - struct drm_framebuffer base;
> >>> -};
> >>> -
> >>> -static inline struct nouveau_framebuffer *
> >>> -nouveau_framebuffer(struct drm_framebuffer *fb)
> >>> -{
> >>> - return container_of(fb, struct nouveau_framebuffer, base);
> >>> -}
> >>> -
> >>> int
> >>> nouveau_framebuffer_new(struct drm_device *dev,
> >>> const struct drm_mode_fb_cmd2 *mode_cmd,
> >>> struct drm_gem_object *gem,
> >>> - struct nouveau_framebuffer **pfb);
> >>> + struct drm_framebuffer **pfb);
> >>> struct nouveau_display {
> >>> void *priv;
> >>> diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
> >>> b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
> >>> index 02b36b44409c..d78bc03ad3b8 100644
> >>> --- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
> >>> +++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
> >>> @@ -312,7 +312,7 @@ nouveau_fbcon_create(struct drm_fb_helper *helper,
> >>> struct nouveau_drm *drm = nouveau_drm(dev);
> >>> struct nvif_device *device = &drm->client.device;
> >>> struct fb_info *info;
> >>> - struct nouveau_framebuffer *fb;
> >>> + struct drm_framebuffer *fb;
> >>> struct nouveau_channel *chan;
> >>> struct nouveau_bo *nvbo;
> >>> struct drm_mode_fb_cmd2 mode_cmd;
> >>> @@ -367,7 +367,7 @@ nouveau_fbcon_create(struct drm_fb_helper *helper,
> >>> }
> >>> /* setup helper */
> >>> - fbcon->helper.fb = &fb->base;
> >>> + fbcon->helper.fb = fb;
> >>> if (!chan)
> >>> info->flags = FBINFO_HWACCEL_DISABLED;
> >>> @@ -393,7 +393,7 @@ nouveau_fbcon_create(struct drm_fb_helper *helper,
> >>> /* To allow resizeing without swapping buffers */
> >>> NV_INFO(drm, "allocated %dx%d fb: 0x%llx, bo %p\n",
> >>> - fb->base.width, fb->base.height, nvbo->bo.offset, nvbo);
> >>> + fb->width, fb->height, nvbo->bo.offset, nvbo);
> >>> vga_switcheroo_client_fb_set(dev->pdev, info);
> >>> return 0;
> >>> @@ -413,18 +413,18 @@ nouveau_fbcon_create(struct drm_fb_helper *helper,
> >>> static int
> >>> nouveau_fbcon_destroy(struct drm_device *dev, struct nouveau_fbdev
> >>> *fbcon)
> >>> {
> >>> - struct nouveau_framebuffer *nouveau_fb =
> >>> nouveau_framebuffer(fbcon->helper.fb);
> >>> + struct drm_framebuffer *fb = fbcon->helper.fb;
> >>> struct nouveau_bo *nvbo;
> >>> drm_fb_helper_unregister_fbi(&fbcon->helper);
> >>> drm_fb_helper_fini(&fbcon->helper);
> >>> - if (nouveau_fb && nouveau_fb->base.obj[0]) {
> >>> - nvbo = nouveau_gem_object(nouveau_fb->base.obj[0]);
> >>> + if (fb && fb->obj[0]) {
> >>> + nvbo = nouveau_gem_object(fb->obj[0]);
> >>> nouveau_vma_del(&fbcon->vma);
> >>> nouveau_bo_unmap(nvbo);
> >>> nouveau_bo_unpin(nvbo);
> >>> - drm_framebuffer_put(&nouveau_fb->base);
> >>> + drm_framebuffer_put(fb);
> >>> }
> >>> return 0;
> >>>
> >> _______________________________________________
> >> Nouveau mailing list
> >> Nouveau@xxxxxxxxxxxxxxxxxxxxx
> >> https://lists.freedesktop.org/mailman/listinfo/nouveau
> _______________________________________________
> Nouveau mailing list
> Nouveau@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/nouveau
_______________________________________________
Nouveau mailing list
Nouveau@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/nouveau
