| Message ID | 1473345868-25453-13-git-send-email-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@ideasonboard.com> > Reviewed-by: Sinclair Yeh <syeh@vmware.com> > --- > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > Cc: VMware Graphics <linux-graphics-maintainer@vmware.com> > Cc: Sinclair Yeh <syeh@vmware.com> > Cc: Thomas Hellstrom <thellstrom@vmware.com> > > 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. -Daniel > > /** > * This code should be conditioned on Screen Objects not being used. > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
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@ideasonboard.com> > > Reviewed-by: Sinclair Yeh <syeh@vmware.com> > > --- > > > > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 12 ++++++++++-- > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > Cc: VMware Graphics <linux-graphics-maintainer@vmware.com> > > Cc: Sinclair Yeh <syeh@vmware.com> > > Cc: Thomas Hellstrom <thellstrom@vmware.com> > > > > 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. > > /** > > > > * This code should be conditioned on Screen Objects not being used.
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@ideasonboard.com> > > > Reviewed-by: Sinclair Yeh <syeh@vmware.com> > > > --- > > > > > > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 12 ++++++++++-- > > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > > > Cc: VMware Graphics <linux-graphics-maintainer@vmware.com> > > > Cc: Sinclair Yeh <syeh@vmware.com> > > > Cc: Thomas Hellstrom <thellstrom@vmware.com> > > > > > > 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@ffwll.ch> > > > > /** > > > > > > * This code should be conditioned on Screen Objects not being used. > > -- > Regards, > > Laurent Pinchart >
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; /** * This code should be conditioned on Screen Objects not being used.