Re: [PATCH 08/12] gma500: Add the core DRM files and headers

[Alan Cox <alan@lxorguk.ukuu.org.uk>]

> So generally we need to provide a userspace interface via ioctls, we
> do this with a shared header file that goes in include/drm/ with the
> Kbuild bits

At the moment the only public API is the generic bits.

> Now I'm assuming psb_drm.h is meant to be this file? but as-is its not
> really what I'd expect,

Not yet.

> If you look at radeon you'll see defines for device specific ioctls,
> same for i915, now I note these tables start at 0 and work their way
> up,

I'll move the ones we want to zero.

> Now if I understand the holes in this are due to some old userspace
> code that is probably broken anyway,

Yep.. but really I can kill them off. The total number of people affected
will be < 10.

> willing to listen to why this is a bad plan, but I'd rather not push
> psb_drm.h into kernel header packages and libdrm if there is a
> conflicting one in existance (or conflicting 6).

Last time I looked there were conflicts between the various IMG based
Intel ones let along with anything else - lost cause so we might as well
do it right.

We could use a new namespace that might be smarter in fact.

> > +#define PSB_BO_FLAG_COMMAND         (1ULL << 52)
> 
> Any reason it start at 52?

That may be obsolete

> 
> > +struct drm_psb_mode_operation_arg {
> > +       u32 obj_id;
> > +       u16 operation;
> > +       struct drm_mode_modeinfo mode;
> > +       void *data;
> > +};
> 
> No void * in ioctl args, no u16 in ioctls args, not really sure what a
> mode "operation" is here either. Try and design the ioctls to avoid
> compat wrapper requirements.
> Maybe move the operation flags up here.

Again I need to see if we can simply kill it off

> > +
> > +struct drm_psb_stolen_memory_arg {
> > +       u32 base;
> > +       u32 size;
> > +};
> 
> Why does someone care about this? is it a workaround for lack of
> decent memory management?

A smart GMA500 aware server can allocate GEM objects from host memory or
from the stolen memory area. Being smart about it really means knowing
how much stolen memory is so it can see how best to use it.

> > +/* Controlling the kernel modesetting buffers */
> > +
> > +#define DRM_PSB_SIZES           0x07
> > +#define DRM_PSB_FUSE_REG       0x08
> > +#define DRM_PSB_DC_STATE       0x0A
> > +#define DRM_PSB_ADB            0x0B
> > +#define DRM_PSB_MODE_OPERATION 0x0C
> > +#define DRM_PSB_STOLEN_MEMORY  0x0D
> > +#define DRM_PSB_REGISTER_RW    0x0E
> 
> Direct read/writing of registers is not something, the regs being hit
> here seem like workarounds for not having good overlay support in the
> kernel perhaps,
> can the new plane stuff help workaround this?

That can go - its basically left for debugging.
 
> So you can probably considered these patches merged at least, I'll
> just keep them in a topic branch which I'll stick into the drm-next
> queue.

Cool.

I'll take the secateurs to the interfaces.

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel