Hi Michel,
Do you have any further comments/suggestions on this patch?
Thanks and Regards,
Vivek
On Fri, 6 Nov 2015 18:56:13 +0900
Michel Dänzer <mic...@daenzer.net> wrote:
> On 06.11.2015 12:08, Vivek Kasireddy wrote:
> > For certain platforms that support rotated scanout buffers,
> > currently, there is no way to create them with the GBM DRI
> > interface. These flags will instruct the DRI driver to create the
> > buffer by setting additional requirements such as tiling mode.
> >
> > v2: Reserve a bit per angle. (Ville and Michel)
> >
> > v3:
> > - Combine all GBM_BO_USE_SCANOUT_ROTATION_* flags into
> > GBM_BO_USE_SCANOUT_ANY macro (Michel)
> > - Pull the code that updates dri_use based on the rotation flag
> > into a separate function.
>
> [...]
>
> > diff --git a/src/gbm/backends/dri/gbm_dri.c
> > b/src/gbm/backends/dri/gbm_dri.c index 57cdeac..741e509 100644
> > --- a/src/gbm/backends/dri/gbm_dri.c
> > +++ b/src/gbm/backends/dri/gbm_dri.c
> > @@ -124,6 +124,20 @@ image_get_buffers(__DRIdrawable *driDrawable,
> > }
> >
> > static void
> > +gbm_to_dri_flag(uint32_t usage,
> > + unsigned *dri_use)
> > +{
> > + if (usage & GBM_BO_USE_SCANOUT)
> > + *dri_use |= __DRI_IMAGE_USE_SCANOUT;
> > + if (usage & GBM_BO_USE_SCANOUT_ROTATION_90)
> > + *dri_use |= __DRI_IMAGE_USE_SCANOUT_ROTATION_90;
> > + if (usage & GBM_BO_USE_SCANOUT_ROTATION_180)
> > + *dri_use |= __DRI_IMAGE_USE_SCANOUT_ROTATION_180;
> > + if (usage & GBM_BO_USE_SCANOUT_ROTATION_270)
> > + *dri_use |= __DRI_IMAGE_USE_SCANOUT_ROTATION_270;
> > +}
>
> I like the idea of this helper function, but it could handle
> GBM_BO_USE_CURSOR and GBM_BO_USE_LINEAR as well. Ideally, there would
> be a separate preparatory change which creates the helper function and
> makes gbm_dri_bo_import and gbm_dri_bo_create use it; then this change
> can just add the new flags in the helper function. If that's too much
> trouble, the handling of the other flags can be moved into the helper
> function in a followup change.
>
>
> > diff --git a/src/gbm/main/gbm.h b/src/gbm/main/gbm.h
> > index 8db2153..4bda089 100644
> > --- a/src/gbm/main/gbm.h
> > +++ b/src/gbm/main/gbm.h
> > @@ -214,6 +214,13 @@ enum gbm_bo_flags {
> > * Buffer is linear, i.e. not tiled.
> > */
> > GBM_BO_USE_LINEAR = (1 << 4),
> > + /**
> > + * Buffer would be rotated and some platforms have additional
> > tiling
> > + * requirements for rotated scanout buffers.
> > + */
> > + GBM_BO_USE_SCANOUT_ROTATION_90 = (1 << 5),
> > + GBM_BO_USE_SCANOUT_ROTATION_180 = (1 << 6),
> > + GBM_BO_USE_SCANOUT_ROTATION_270 = (1 << 7),
> > };
>
> Hmm, we should probably explicitly specify the orientation of the 90
> and 270 degree rotations. Clockwise? (Same in patch 1)
>
>
> > @@ -240,6 +247,10 @@ gbm_bo_create(struct gbm_device *gbm,
> > #define GBM_BO_IMPORT_WL_BUFFER 0x5501
> > #define GBM_BO_IMPORT_EGL_IMAGE 0x5502
> > #define GBM_BO_IMPORT_FD 0x5503
> > +#define GBM_BO_USE_SCANOUT_ANY (GBM_BO_USE_SCANOUT
> > | \
> > + GBM_BO_USE_SCANOUT_ROTATION_90 | \
> > + GBM_BO_USE_SCANOUT_ROTATION_180 | \
> > + GBM_BO_USE_SCANOUT_ROTATION_170)
> >
> > struct gbm_import_fd_data {
> > int fd;
> >
>
> Please add the define after the definition of enum gbm_bo_flags
> instead of here.
>
>
> Other than that, these are starting to look pretty good to me.
>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
