Re: [PATCH 3/3] drm/vmwgfx: Delay pinning fbdev framebuffer until after mode set

[Sinclair Yeh <syeh@vmware.com>]

Argh, you're right.  I'll send out a patch.  Thanks for catching this!

Sinclair

On Mon, Jul 04, 2016 at 04:51:52PM +0100, Emil Velikov wrote:
> Hi Sinclair,
> 
> On 1 July 2016 at 19:24,  <syeh@vmware.com> wrote:
> > From: Sinclair Yeh <syeh@vmware.com>
> >
> > For the Screen Object display unit, we need to reserve a
> > guest-invisible region equal to the size of the framebuffer for
> > the host.  This region can only be reserved in VRAM, whereas
> > the guest-visible framebuffer can be reserved in either VRAM or
> > GMR.
> >
> > As such priority should be given to the guest-invisible
> > region otherwise in a limited VRAM situation, we can fail to
> > allocate this region.
> >
> > This patch makes it so that vmw_sou_backing_alloc() is called
> > before the framebuffer is pinned.
> >
> > Signed-off-by: Sinclair Yeh <syeh@vmware.com>
> > Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
> > Cc: <stable@vger.kernel.org>
> > ---
> > This is the last patch of a 3-patch series to fix console black
> > screen issue on Ubuntu 16.04 server
> > ---
> >  drivers/gpu/drm/vmwgfx/vmwgfx_fb.c | 47 ++++++++++++++++++++------------------
> >  1 file changed, 25 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
> > index 679a4cb..66eaa30 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
> > @@ -517,28 +517,6 @@ static int vmw_fb_kms_framebuffer(struct fb_info *info)
> >
> >         par->set_fb = &vfb->base;
> >
> > -       if (!par->bo_ptr) {
> > -               /*
> > -                * Pin before mapping. Since we don't know in what placement
> > -                * to pin, call into KMS to do it for us.
> > -                */
> > -               ret = vfb->pin(vfb);
> > -               if (ret) {
> > -                       DRM_ERROR("Could not pin the fbdev framebuffer.\n");
> > -                       return ret;
> > -               }
> > -
> > -               ret = ttm_bo_kmap(&par->vmw_bo->base, 0,
> > -                                 par->vmw_bo->base.num_pages, &par->map);
> > -               if (ret) {
> > -                       vfb->unpin(vfb);
> > -                       DRM_ERROR("Could not map the fbdev framebuffer.\n");
> > -                       return ret;
> > -               }
> > -
> > -               par->bo_ptr = ttm_kmap_obj_virtual(&par->map, &par->bo_iowrite);
> > -       }
> > -
> >         return 0;
> >  }
> >
> > @@ -601,6 +579,31 @@ static int vmw_fb_set_par(struct fb_info *info)
> >         if (ret)
> >                 goto out_unlock;
> >
> > +       if (!par->bo_ptr) {
> > +               struct vmw_framebuffer *vfb = vmw_framebuffer_to_vfb(set.fb);
> > +
> > +               /*
> > +                * Pin before mapping. Since we don't know in what placement
> > +                * to pin, call into KMS to do it for us.
> > +                */
> > +               ret = vfb->pin(vfb);
> > +               if (ret) {
> > +                       DRM_ERROR("Could not pin the fbdev framebuffer.\n");
> > +                       return ret;
> > +               }
> > +
> > +               ret = ttm_bo_kmap(&par->vmw_bo->base, 0,
> > +                                 par->vmw_bo->base.num_pages, &par->map);
> > +               if (ret) {
> > +                       vfb->unpin(vfb);
> > +                       DRM_ERROR("Could not map the fbdev framebuffer.\n");
> > +                       return ret;
> > +               }
> > +
> > +               par->bo_ptr = ttm_kmap_obj_virtual(&par->map, &par->bo_iowrite);
> > +       }
> > +
> > +
> Seems like you forgot to update the error paths. Thankfully we
> shouldn't be reaching those too often, if at all.
> 
> -Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel