On Wed, Jun 11, 2014 at 10:43:30AM -0700, Stéphane Marchesin wrote:
> On Tue, Jun 10, 2014 at 4:04 AM, Thierry Reding
> <thierry.reding@xxxxxxxxx> wrote:
> > From: Thierry Reding <treding@xxxxxxxxxx>
> >
> > Currently the tiling parameters of buffer objects can only be set at
> > allocation time, and only a single tiled mode is supported. This new
> > DRM_TEGRA_GEM_SET_TILING IOCTL allows more modes to be set and also
> > allows the tiling mode to be changed after the allocation. This will
> > enable the Tegra DRM driver to import buffers from a GPU and directly
> > scan them out by configuring the display controller appropriately.
>
> I was wondering why not support the new tiling modes on creation as
> well, I guess because you don't have room for height in the creation
> ioctl?
Yes, back at the time when the GEM_CREATE IOCTL was added we only left
enough room for "flags", so it'll be difficult to add support for the
newer tiling modes.
One possibility that I see is to extend struct drm_tegra_gem_create in a
backwards-compatible way and make new fields dependent on some new flag,
somewhat like this:
#define DRM_TEGRA_GEM_CREATE_BLOCK_LINEAR (1 << 3)
struct drm_tegra_gem_create {
__u64 size;
__u32 flags;
__u32 handle;
__u32 height;
__u32 pad;
};
Then the DRM driver can parse the height field only if the BLOCK_LINEAR
flag is set.
To be honest, though, I'd prefer to deprecate the flags at creation if
these patches get merged so that we can have a standard way of setting
these rather than a few.
Do you see any particular advantages in specifying the tiling mode at
creation time?
> > diff --git a/include/uapi/drm/tegra_drm.h b/include/uapi/drm/tegra_drm.h
> > index b75482112428..0829f75eb986 100644
> > --- a/include/uapi/drm/tegra_drm.h
> > +++ b/include/uapi/drm/tegra_drm.h
> > @@ -129,6 +129,27 @@ struct drm_tegra_submit {
> > __u32 reserved[5]; /* future expansion */
> > };
> >
> > +#define DRM_TEGRA_GEM_TILING_MODE_PITCH 0
> > +#define DRM_TEGRA_GEM_TILING_MODE_TILED 1
> > +#define DRM_TEGRA_GEM_TILING_MODE_BLOCK 2
> > +
> > +struct drm_tegra_gem_set_tiling {
> > + /* input */
> > + __u32 handle;
> > + __u32 mode;
> > + __u32 value;
>
> This seems to only be used for height, if so should it be called height?
The idea here was to have a generic name here so that it could be more
easily extended if/when newer tiling modes are introduced.
Thierry
Attachment:
pgpcDbVxjPQU4.pgp
Description: PGP signature
