On Thu, Nov 17, 2016 at 07:52:23PM +0200, Laurent Pinchart wrote:
> Hi Ville,
>
> Thank you for the patch.
>
> On Thursday 17 Nov 2016 18:14:13 ville.syrjala@xxxxxxxxxxxxxxx wrote:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > Stuff something semi-reasonable into fb->pixel_format. I had to guess
> > as to which formats we should pick. Did I guess correctly?
> >
> > We can't quite use drm_mode_legacy_fb_format() due to the ARGB1555
> > vs. XRGB155 mess. However use of 'A' formats should imply per-pixel
> > alpha blending as far as KMS is concerned so I'm not at all sure we
> > want to have any 'A' formats exposed as opposed to just 'X' formats.
> > OTOH vmvgfx doesn't do planes, and so the core won't enforce that
> > these formats match any list of supported formats, so the choice
> > shouldn't be super important at this point.
>
> The vmgfx driver should really be converted to using struct drm_mode_fb_cmd2
> through the whole code instead of converting struct drm_mode_fb_cmd2 to struct
> drm_mode_fb_cmd in vmw_kms_fb_create(). I gave it a go a few months back but
> the structure is passed around so many functions that conversion requires more
> knowledge about the driver than I have. I don't expect you to fix that, but it
> would be nice if Sinclair or Thomas could give it a go.
Indeed. Would be nice to bring all drivers into the fold. I didn't
want to start down this particular hole with the series already
too big as is.
>
> > Cc: linux-graphics-maintainer@xxxxxxxxxx
> > Cc: Sinclair Yeh <syeh@xxxxxxxxxx>
> > Cc: Thomas Hellstrom <thellstrom@xxxxxxxxxx>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 23 ++++++++++++++++++++++-
> > 1 file changed, 22 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c index 7d92ab56961b..5788913ca8f9
> > 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > @@ -524,6 +524,7 @@ static int vmw_kms_new_framebuffer_surface(struct
> > vmw_private *dev_priv, struct drm_device *dev = dev_priv->dev;
> > struct vmw_framebuffer_surface *vfbs;
> > enum SVGA3dSurfaceFormat format;
> > + u32 pixel_format;
> > int ret;
> >
> > /* 3D is only supported on HWv8 and newer hosts */
> > @@ -548,17 +549,22 @@ static int vmw_kms_new_framebuffer_surface(struct
> > vmw_private *dev_priv, return -EINVAL;
> > }
> >
> > + /* FIXME 'A' format implies per-pixel alpha blending for KMS */
> > switch (mode_cmd->depth) {
> > case 32:
> > + pixel_format = DRM_FORMAT_ARGB8888;
> > format = SVGA3D_A8R8G8B8;
> > break;
> > case 24:
> > + pixel_format = DRM_FORMAT_XRGB8888;
> > format = SVGA3D_X8R8G8B8;
> > break;
> > case 16:
> > + pixel_format = DRM_FORMAT_RGB565;
> > format = SVGA3D_R5G6B5;
> > break;
> > case 15:
> > + pixel_format = DRM_FORMAT_ARGB1555;
> > format = SVGA3D_A1R5G5B5;
> > break;
> > default:
> > @@ -582,7 +588,8 @@ static int vmw_kms_new_framebuffer_surface(struct
> > vmw_private *dev_priv, }
> >
> > vfbs->base.base.dev = dev;
> > - /* XXX get the first 3 from the surface info */
> > + /* XXX get the first 4 from the surface info */
> > + vfbs->base.base.pixel_format = pixel_format;
> > vfbs->base.base.bits_per_pixel = mode_cmd->bpp;
> > vfbs->base.base.pitches[0] = mode_cmd->pitch;
> > vfbs->base.base.depth = mode_cmd->depth;
> > @@ -834,6 +841,7 @@ static int vmw_kms_new_framebuffer_dmabuf(struct
> > vmw_private *dev_priv, struct drm_device *dev = dev_priv->dev;
> > struct vmw_framebuffer_dmabuf *vfbd;
> > unsigned int requested_size;
> > + u32 pixel_format;
> > int ret;
> >
> > requested_size = mode_cmd->height * mode_cmd->pitch;
> > @@ -852,6 +860,12 @@ static int vmw_kms_new_framebuffer_dmabuf(struct
> > vmw_private *dev_priv, if (mode_cmd->bpp == 32)
> > break;
> >
> > + /* FIXME 'A' format implies per-pixel alpha blending
> for KMS */
> > + if (mode_cmd->depth == 32)
> > + pixel_format = DRM_FORMAT_ARGB8888;
> > + else
> > + pixel_format = DRM_FORMAT_XRGB8888;
> > +
> > DRM_ERROR("Invalid color depth/bbp: %d %d\n",
> > mode_cmd->depth, mode_cmd->bpp);
> > return -EINVAL;
> > @@ -861,6 +875,12 @@ static int vmw_kms_new_framebuffer_dmabuf(struct
> > vmw_private *dev_priv, if (mode_cmd->bpp == 16)
> > break;
> >
> > + /* FIXME 'A' format implies per-pixel alpha blending
> for KMS */
> > + if (mode_cmd->depth == 16)
> > + pixel_format = DRM_FORMAT_RGB565;
> > + else
> > + pixel_format = DRM_FORMAT_ARGB1555;
> > +
> > DRM_ERROR("Invalid color depth/bbp: %d %d\n",
> > mode_cmd->depth, mode_cmd->bpp);
> > return -EINVAL;
> > @@ -877,6 +897,7 @@ static int vmw_kms_new_framebuffer_dmabuf(struct
> > vmw_private *dev_priv, }
> >
> > vfbd->base.base.dev = dev;
> > + vfbd->base.base.pixel_format = pixel_format;
> > vfbd->base.base.bits_per_pixel = mode_cmd->bpp;
> > vfbd->base.base.pitches[0] = mode_cmd->pitch;
> > vfbd->base.base.depth = mode_cmd->depth;
>
> --
> Regards,
>
> Laurent Pinchart
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
