Re: [PATCH] drm: Allow also control clients to check the drm version

From: Thomas Hellstrom <thellstrom@vmware.com>
To: David Herrmann <dh.herrmann@gmail.com>
Cc: "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH] drm: Allow also control clients to check the drm version
Date: Wed, 16 Sep 2015 17:03:25 +0200	[thread overview]
Message-ID: <55F984BD.1060602@vmware.com> (raw)
In-Reply-To: <CANq1E4Rqt1_Nr0WnVPNF+gnPS5oeDwXsHGVDvrN0bbY1qn2BFw@mail.gmail.com>

Hi, David,

On 09/16/2015 04:35 PM, David Herrmann wrote:
> Hi
>
> On Tue, Sep 15, 2015 at 10:11 AM, Thomas Hellstrom
> <thellstrom@vmware.com> wrote:
>> This should be harmless.
>> Vmware will, due to old infrastructure reasons, be using a privileged
>> control client to supply GUI layout information rather than obtaining
>> it from the device. That control client will be needing access to DRM
>> version information.
> I didn't add it to render-nodes as I wasn't aware of any driver still
> using the version-information.

In fact you did add it to render nodes (commit 3d3b78c), but this is for
control nodes.

>  I'm not a big fan on relying on magic
> numbers, as it doesn't work well with (stable) backports, but if you
> need it for backwards-compat on vmwgfx, I'm fine with it. But I
> certainly don't want to encourage new driver authors to use it.

We view the driver version information as user-space api version
information, which I can't see changing
with stable backports (except perhaps in very unlikely situations).
Of course, there are other ways to signal feature availability, but
we've traditionally been checking version minor.

>
>> Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
>> Reviewed-by: Brian Paul <brianp@vmware.com>
>> Reviewed-by: Sinclair Yeh <syeh@vmware.com>
>> ---
>>  drivers/gpu/drm/drm_ioctl.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
>> index 9a860ca..d93e737 100644
>> --- a/drivers/gpu/drm/drm_ioctl.c
>> +++ b/drivers/gpu/drm/drm_ioctl.c
>> @@ -520,7 +520,8 @@ EXPORT_SYMBOL(drm_ioctl_permit);
>>
>>  /** Ioctl table */
>>  static const struct drm_ioctl_desc drm_ioctls[] = {
>> -       DRM_IOCTL_DEF(DRM_IOCTL_VERSION, drm_version, DRM_UNLOCKED|DRM_RENDER_ALLOW),
>> +       DRM_IOCTL_DEF(DRM_IOCTL_VERSION, drm_version,
>> +                     DRM_UNLOCKED|DRM_RENDER_ALLOW|DRM_CONTROL_ALLOW),
> Why the line-break? None of the other ioctl definitions cares for the
> 80ch limit. I'd prefer keeping this uniform.

OK, I'll fix that up.

>
> Acked-by: David Herrmann <dh.herrmann@gmail.com>
>
> Thanks
> David

Thanks,
Thomas

>
>>         DRM_IOCTL_DEF(DRM_IOCTL_GET_UNIQUE, drm_getunique, 0),
>>         DRM_IOCTL_DEF(DRM_IOCTL_GET_MAGIC, drm_getmagic, 0),
>>         DRM_IOCTL_DEF(DRM_IOCTL_IRQ_BUSID, drm_irq_by_busid, DRM_MASTER|DRM_ROOT_ONLY),
>> --
>> 2.1.0
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.freedesktop.org_mailman_listinfo_dri-2Ddevel&d=BQIBaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=vpukPkBtpoNQp2IUKuFviOmPNYWVKmen3Jeeu55zmEA&m=-dqJN9pZD2sav6M76SFMIM359ZWmPz3fLT0oU30MVRw&s=ICMYbx449lJwRcYDljH6Z8Zp9SQO6mX71E9Y9OgL_SA&e= 

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