On Fri, Sep 23, 2016 at 03:40:17PM +0300, Laurent Pinchart wrote:
> Hi Daniel,
>
> On Wednesday 21 Sep 2016 09:31:58 Daniel Vetter wrote:
> > On Thu, Sep 08, 2016 at 05:44:26PM +0300, Laurent Pinchart wrote:
> > > The driver is the last users of the drm_fb_get_bpp_depth() function. It
> > > should ideally be converted to use struct drm_mode_fb_cmd2 instead of
> > > the legacy struct drm_mode_fb_cmd internally, but that will require
> > > broad changes across the code base. As a first step, replace
> > > drm_fb_get_bpp_depth() with drm_format_info() in order to stop exporting
> > > the function to drivers.
> > >
> > > The new DRM_ERROR() message comes from the vmw_create_dmabuf_proxy(),
> > > vmw_kms_new_framebuffer_surface() and vmw_kms_new_framebuffer_dmabuf()
> > > functions that currently print an error if the pixel format is
> > > unsupported.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > > Reviewed-by: Sinclair Yeh <syeh@xxxxxxxxxx>
> > > ---
> > >
> > > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 12 ++++++++++--
> > > 1 file changed, 10 insertions(+), 2 deletions(-)
> > >
> > > Cc: VMware Graphics <linux-graphics-maintainer@xxxxxxxxxx>
> > > Cc: Sinclair Yeh <syeh@xxxxxxxxxx>
> > > Cc: Thomas Hellstrom <thellstrom@xxxxxxxxxx>
> > >
> > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > > b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index bf28ccc150df..c965514b82be
> > > 100644
> > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > > @@ -980,14 +980,22 @@ static struct drm_framebuffer
> > > *vmw_kms_fb_create(struct drm_device *dev,>
> > > struct vmw_dma_buffer *bo = NULL;
> > > struct ttm_base_object *user_obj;
> > > struct drm_mode_fb_cmd mode_cmd;
> > >
> > > + const struct drm_format_info *info;
> > >
> > > int ret;
> > >
> > > + info = drm_format_info(mode_cmd2->pixel_format);
> > > + if (!info || !info->depth) {
> > > + DRM_ERROR("Unsupported framebuffer format %s\n",
> > > + drm_get_format_name(mode_cmd2->pixel_format));
> > > + return ERR_PTR(-EINVAL);
> > > + }
> > > +
> > >
> > > mode_cmd.width = mode_cmd2->width;
> > > mode_cmd.height = mode_cmd2->height;
> > > mode_cmd.pitch = mode_cmd2->pitches[0];
> > > mode_cmd.handle = mode_cmd2->handles[0];
> > >
> > > - drm_fb_get_bpp_depth(mode_cmd2->pixel_format, &mode_cmd.depth,
> > > - &mode_cmd.bpp);
> > > + mode_cmd.depth = info->depth;
> > > + mode_cmd.bpp = info->cpp[0] * 8;
> >
> > I think this should use drm_helper_mode_fill_fb_struct instead.
>
> I would do that if there was a struct drm_framebuffer to fill, but this piece
> of code converts from struct drm_mode_fb_cmd2 to drm_mode_fb_cmd that is then
> used all over the place internally. This should be fixed, but I think that's
> out of scope for this patch series.
Oh right, I didn't realize that ...
Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
>
> > > /**
> > >
> > > * This code should be conditioned on Screen Objects not being used.
>
> --
> Regards,
>
> Laurent Pinchart
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
