Re: [PATCH] drm: deprecate driver date

2024-05-10 Thread Steven Price
On 10/05/2024 10:13, Jani Nikula wrote:
> On Thu, 09 May 2024, Steven Price  wrote:
>> On 29/04/2024 17:43, Jani Nikula wrote:
>>> The driver date serves no useful purpose, because it's hardly ever
>>> updated. The information is misleading at best.
>>>
>>> As described in Documentation/gpu/drm-internals.rst:
>>>
>>>   The driver date, formatted as MMDD, is meant to identify the date
>>>   of the latest modification to the driver. However, as most drivers
>>>   fail to update it, its value is mostly useless. The DRM core prints it
>>>   to the kernel log at initialization time and passes it to userspace
>>>   through the DRM_IOCTL_VERSION ioctl.
>>>
>>> Stop printing the driver date at init, and start returning the empty
>>> string "" as driver date through the DRM_IOCTL_VERSION ioctl.
>>
>> I agree with the idea of this, unfortuantly it breaks user space :(
>>
>> It's a bug in libdrm, but given this breaks existing user space I think
>> we'll need to revert/reconsider.
>>
>> The issue is in drmGetVersion() [1]:
>>
>>> if (version->date_len)  
>>>  
>>> version->date= drmMalloc(version->date_len + 1);
>>>  
>>
>> So if date_len == 0, then version->date isn't populated (and isn't
>> initialized at all). But then later on in drmCopyVersion() [2] the
>> (unset) version->date is passed to strdup():
>>
>>> static void drmCopyVersion(drmVersionPtr d, const drm_version_t *s) 
>>>  
>>> {   
>>>  
>>> d->version_major  = s->version_major;   
>>>  
>>> d->version_minor  = s->version_minor;   
>>>  
>>> d->version_patchlevel = s->version_patchlevel;  
>>>  
>>> d->name_len   = s->name_len;
>>>  
>>> d->name   = strdup(s->name);
>>>  
>>> d->date_len   = s->date_len;
>>>  
>>> d->date   = strdup(s->date);
>>>  
>>> d->desc_len   = s->desc_len;
>>>  
>>> d->desc   = strdup(s->desc);
>>>  
>>> }   
>>>  
>>
>> Which then segfaults if the uninitialized value points off somewhere
>> bad. And this does happen (my test setup reproduced this).
> 
> Thanks for the report!
> 
>> A simple fix is to make sure the string isn't empty - so return
>> "unknown" or just a space, or even "\0".
> 
> I don't think "\0" works, because strlen() will still return 0 for it.

Ah, true - you'd have to hack up drm_copy_field() to someone return a
length of 1 in this case. And I'd be a little worried that it would
break something else...

> I went ahead with "0", because that's already been used by virtio until
> now. Fix at [1].

Yep, that seems like the best solution.

Thanks,

Steve

> BR,
> Jani.
> 
> 
> [1] https://lore.kernel.org/r/20240510090951.3398882-1-jani.nik...@intel.com
> 
> 


Re: [PATCH] drm: deprecate driver date

2024-05-10 Thread Jani Nikula
On Thu, 09 May 2024, Steven Price  wrote:
> On 29/04/2024 17:43, Jani Nikula wrote:
>> The driver date serves no useful purpose, because it's hardly ever
>> updated. The information is misleading at best.
>> 
>> As described in Documentation/gpu/drm-internals.rst:
>> 
>>   The driver date, formatted as MMDD, is meant to identify the date
>>   of the latest modification to the driver. However, as most drivers
>>   fail to update it, its value is mostly useless. The DRM core prints it
>>   to the kernel log at initialization time and passes it to userspace
>>   through the DRM_IOCTL_VERSION ioctl.
>> 
>> Stop printing the driver date at init, and start returning the empty
>> string "" as driver date through the DRM_IOCTL_VERSION ioctl.
>
> I agree with the idea of this, unfortuantly it breaks user space :(
>
> It's a bug in libdrm, but given this breaks existing user space I think
> we'll need to revert/reconsider.
>
> The issue is in drmGetVersion() [1]:
>
>> if (version->date_len)   
>> 
>> version->date= drmMalloc(version->date_len + 1); 
>> 
>
> So if date_len == 0, then version->date isn't populated (and isn't
> initialized at all). But then later on in drmCopyVersion() [2] the
> (unset) version->date is passed to strdup():
>
>> static void drmCopyVersion(drmVersionPtr d, const drm_version_t *s)  
>> 
>> {
>> 
>> d->version_major  = s->version_major;
>> 
>> d->version_minor  = s->version_minor;
>> 
>> d->version_patchlevel = s->version_patchlevel;   
>> 
>> d->name_len   = s->name_len; 
>> 
>> d->name   = strdup(s->name); 
>> 
>> d->date_len   = s->date_len; 
>> 
>> d->date   = strdup(s->date); 
>> 
>> d->desc_len   = s->desc_len; 
>> 
>> d->desc   = strdup(s->desc); 
>> 
>> }
>> 
>
> Which then segfaults if the uninitialized value points off somewhere
> bad. And this does happen (my test setup reproduced this).

Thanks for the report!

> A simple fix is to make sure the string isn't empty - so return
> "unknown" or just a space, or even "\0".

I don't think "\0" works, because strlen() will still return 0 for it.

I went ahead with "0", because that's already been used by virtio until
now. Fix at [1].

BR,
Jani.


[1] https://lore.kernel.org/r/20240510090951.3398882-1-jani.nik...@intel.com


>
> Steve
>
> [1]
> https://gitlab.freedesktop.org/mesa/drm/-/blob/7c5c742de8a8b577654964635f05d7033c92ee53/xf86drm.c#L1393
>
> [2]
> https://gitlab.freedesktop.org/mesa/drm/-/blob/7c5c742de8a8b577654964635f05d7033c92ee53/xf86drm.c#L1352
>
>
>> The driver date initialization in drivers and the struct drm_driver date
>> member can be removed in follow-up.
>> 
>> Signed-off-by: Jani Nikula 
>> 
>> ---
>> 
>> The below approximates when each driver's date was last updated.
>> 
>> $ git grepblame "\(\.date = \".*\"\|#define.*DRIVER_DATE\)" -- drivers/gpu 
>> drivers/accel
>> fe77368c0f3e0 drivers/accel/habanalabs/common/habanalabs_drv.c 94 (Tomer 
>> Tayar 2023-02-19 11:58:46 +0200 104).date = "20190505",
>> 35b137630f08d drivers/accel/ivpu/ivpu_drv.h 20 (Jacek Lawrynowicz 2023-01-17 
>> 10:27:17 +0100 24) #define DRIVER_DATE "20230117"
>> d38ceaf99ed01 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.h 43 (Alex Deucher 
>> 2015-04-20 16:55:21 -0400 43) #define DRIVER_DATE "20150101"
>> 61f1c4a8ab757 drivers/gpu/drm/arm/display/komeda/komeda_kms.c 51 (james qian 
>> wang (Arm Technology China) 2019-01-03 11:41:30 + 64)   .date = 
>> "20181101",
>> 8e22d79240d95 drivers/gpu/drm/arm/hdlcd_drv.c 343 (Liviu Dudau 2015-04-02 
>> 19:48:39 +0100 234).date = "20151021",
>> ad49f8602fe88 drivers/gpu/drm/arm/malidp_drv.c 232 (Liviu Dudau 2016-03-07 
>> 10:00:53 + 571)   .date = "20160106",
>> 4f2a8f5898ecd drivers/gpu/drm/aspeed/aspeed_gfx_drv.c 208 (Joel Stanley 
>> 2019-04-03 10:49:08 +1030 253)   .date = "20180319",
>> 312fec1405dd5 drivers/gpu/drm/ast/ast_drv.h 46 (Dave Airlie 2012-02-29 
>> 13:40:04 + 46) #define DRIVER_DATE"20120228"
>> 1a396789f65a2 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c 504 (Boris 
>> Brezillon 2015-01-06 11:13:28 +0100 741)   .date = "20141504",
>> 9913f74fe1570 drivers/gpu/drm/exynos/exynos_drm_drv.c 37 (Marek Szyprowski 
>> 2018-05-10 08:46:36 +0900 37) #define DRIVER_DATE "20180330"
>> f90cd811ae7a3 drivers/gpu/drm/gma500/psb_drv.h 43 (Arthur Borsboom 

Re: [PATCH] drm: deprecate driver date

2024-05-09 Thread Steven Price
On 29/04/2024 17:43, Jani Nikula wrote:
> The driver date serves no useful purpose, because it's hardly ever
> updated. The information is misleading at best.
> 
> As described in Documentation/gpu/drm-internals.rst:
> 
>   The driver date, formatted as MMDD, is meant to identify the date
>   of the latest modification to the driver. However, as most drivers
>   fail to update it, its value is mostly useless. The DRM core prints it
>   to the kernel log at initialization time and passes it to userspace
>   through the DRM_IOCTL_VERSION ioctl.
> 
> Stop printing the driver date at init, and start returning the empty
> string "" as driver date through the DRM_IOCTL_VERSION ioctl.

I agree with the idea of this, unfortuantly it breaks user space :(

It's a bug in libdrm, but given this breaks existing user space I think
we'll need to revert/reconsider.

The issue is in drmGetVersion() [1]:

> if (version->date_len)
>
> version->date= drmMalloc(version->date_len + 1);  
>

So if date_len == 0, then version->date isn't populated (and isn't
initialized at all). But then later on in drmCopyVersion() [2] the
(unset) version->date is passed to strdup():

> static void drmCopyVersion(drmVersionPtr d, const drm_version_t *s)   
>
> { 
>
> d->version_major  = s->version_major; 
>
> d->version_minor  = s->version_minor; 
>
> d->version_patchlevel = s->version_patchlevel;
>
> d->name_len   = s->name_len;  
>
> d->name   = strdup(s->name);  
>
> d->date_len   = s->date_len;  
>
> d->date   = strdup(s->date);  
>
> d->desc_len   = s->desc_len;  
>
> d->desc   = strdup(s->desc);  
>
> } 
>

Which then segfaults if the uninitialized value points off somewhere
bad. And this does happen (my test setup reproduced this).

A simple fix is to make sure the string isn't empty - so return
"unknown" or just a space, or even "\0".

Steve

[1]
https://gitlab.freedesktop.org/mesa/drm/-/blob/7c5c742de8a8b577654964635f05d7033c92ee53/xf86drm.c#L1393

[2]
https://gitlab.freedesktop.org/mesa/drm/-/blob/7c5c742de8a8b577654964635f05d7033c92ee53/xf86drm.c#L1352


> The driver date initialization in drivers and the struct drm_driver date
> member can be removed in follow-up.
> 
> Signed-off-by: Jani Nikula 
> 
> ---
> 
> The below approximates when each driver's date was last updated.
> 
> $ git grepblame "\(\.date = \".*\"\|#define.*DRIVER_DATE\)" -- drivers/gpu 
> drivers/accel
> fe77368c0f3e0 drivers/accel/habanalabs/common/habanalabs_drv.c 94 (Tomer 
> Tayar 2023-02-19 11:58:46 +0200 104) .date = "20190505",
> 35b137630f08d drivers/accel/ivpu/ivpu_drv.h 20 (Jacek Lawrynowicz 2023-01-17 
> 10:27:17 +0100 24) #define DRIVER_DATE "20230117"
> d38ceaf99ed01 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.h 43 (Alex Deucher 
> 2015-04-20 16:55:21 -0400 43) #define DRIVER_DATE  "20150101"
> 61f1c4a8ab757 drivers/gpu/drm/arm/display/komeda/komeda_kms.c 51 (james qian 
> wang (Arm Technology China) 2019-01-03 11:41:30 + 64).date = 
> "20181101",
> 8e22d79240d95 drivers/gpu/drm/arm/hdlcd_drv.c 343 (Liviu Dudau 2015-04-02 
> 19:48:39 +0100 234) .date = "20151021",
> ad49f8602fe88 drivers/gpu/drm/arm/malidp_drv.c 232 (Liviu Dudau 2016-03-07 
> 10:00:53 + 571).date = "20160106",
> 4f2a8f5898ecd drivers/gpu/drm/aspeed/aspeed_gfx_drv.c 208 (Joel Stanley 
> 2019-04-03 10:49:08 +1030 253).date = "20180319",
> 312fec1405dd5 drivers/gpu/drm/ast/ast_drv.h 46 (Dave Airlie 2012-02-29 
> 13:40:04 + 46) #define DRIVER_DATE "20120228"
> 1a396789f65a2 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c 504 (Boris 
> Brezillon 2015-01-06 11:13:28 +0100 741).date = "20141504",
> 9913f74fe1570 drivers/gpu/drm/exynos/exynos_drm_drv.c 37 (Marek Szyprowski 
> 2018-05-10 08:46:36 +0900 37) #define DRIVER_DATE  "20180330"
> f90cd811ae7a3 drivers/gpu/drm/gma500/psb_drv.h 43 (Arthur Borsboom 2014-03-15 
> 22:12:17 +0100 29) #define DRIVER_DATE "20140314"
> 1053d01864937 drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c 1070 (Xu YiPing 
> 2019-08-20 23:06:19 + 930)  .date = "20150718",
> 76c56a5affeba drivers/gpu/drm/hyperv/hyperv_drm_drv.c 22 (Deepak Rawat 
> 2021-05-27 04:22:28 -0700 22) #define DRIVER_DATE "2020"
> 3570bd989acc6 drivers/gpu/drm/i915/i915_driver.h 18 (Jani Nikula 2023-09-29 
> 12:43:23 +0300 18) 

Re: [PATCH] drm: deprecate driver date

2024-05-08 Thread Jani Nikula
On Mon, 29 Apr 2024, Jani Nikula  wrote:
> The driver date serves no useful purpose, because it's hardly ever
> updated. The information is misleading at best.
>
> As described in Documentation/gpu/drm-internals.rst:
>
>   The driver date, formatted as MMDD, is meant to identify the date
>   of the latest modification to the driver. However, as most drivers
>   fail to update it, its value is mostly useless. The DRM core prints it
>   to the kernel log at initialization time and passes it to userspace
>   through the DRM_IOCTL_VERSION ioctl.
>
> Stop printing the driver date at init, and start returning the empty
> string "" as driver date through the DRM_IOCTL_VERSION ioctl.
>
> The driver date initialization in drivers and the struct drm_driver date
> member can be removed in follow-up.
>
> Signed-off-by: Jani Nikula 

Pushed to drm-misc-next, thanks for the reviews and acks.

BR,
Jani.

>
> ---
>
> The below approximates when each driver's date was last updated.
>
> $ git grepblame "\(\.date = \".*\"\|#define.*DRIVER_DATE\)" -- drivers/gpu 
> drivers/accel
> fe77368c0f3e0 drivers/accel/habanalabs/common/habanalabs_drv.c 94 (Tomer 
> Tayar 2023-02-19 11:58:46 +0200 104) .date = "20190505",
> 35b137630f08d drivers/accel/ivpu/ivpu_drv.h 20 (Jacek Lawrynowicz 2023-01-17 
> 10:27:17 +0100 24) #define DRIVER_DATE "20230117"
> d38ceaf99ed01 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.h 43 (Alex Deucher 
> 2015-04-20 16:55:21 -0400 43) #define DRIVER_DATE  "20150101"
> 61f1c4a8ab757 drivers/gpu/drm/arm/display/komeda/komeda_kms.c 51 (james qian 
> wang (Arm Technology China) 2019-01-03 11:41:30 + 64).date = 
> "20181101",
> 8e22d79240d95 drivers/gpu/drm/arm/hdlcd_drv.c 343 (Liviu Dudau 2015-04-02 
> 19:48:39 +0100 234) .date = "20151021",
> ad49f8602fe88 drivers/gpu/drm/arm/malidp_drv.c 232 (Liviu Dudau 2016-03-07 
> 10:00:53 + 571).date = "20160106",
> 4f2a8f5898ecd drivers/gpu/drm/aspeed/aspeed_gfx_drv.c 208 (Joel Stanley 
> 2019-04-03 10:49:08 +1030 253).date = "20180319",
> 312fec1405dd5 drivers/gpu/drm/ast/ast_drv.h 46 (Dave Airlie 2012-02-29 
> 13:40:04 + 46) #define DRIVER_DATE "20120228"
> 1a396789f65a2 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c 504 (Boris 
> Brezillon 2015-01-06 11:13:28 +0100 741).date = "20141504",
> 9913f74fe1570 drivers/gpu/drm/exynos/exynos_drm_drv.c 37 (Marek Szyprowski 
> 2018-05-10 08:46:36 +0900 37) #define DRIVER_DATE  "20180330"
> f90cd811ae7a3 drivers/gpu/drm/gma500/psb_drv.h 43 (Arthur Borsboom 2014-03-15 
> 22:12:17 +0100 29) #define DRIVER_DATE "20140314"
> 1053d01864937 drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c 1070 (Xu YiPing 
> 2019-08-20 23:06:19 + 930)  .date = "20150718",
> 76c56a5affeba drivers/gpu/drm/hyperv/hyperv_drm_drv.c 22 (Deepak Rawat 
> 2021-05-27 04:22:28 -0700 22) #define DRIVER_DATE "2020"
> 3570bd989acc6 drivers/gpu/drm/i915/i915_driver.h 18 (Jani Nikula 2023-09-29 
> 12:43:23 +0300 18) #define DRIVER_DATE"20230929"
> 4babef0708656 drivers/gpu/drm/imagination/pvr_drv.h 12 (Sarah Walker 
> 2023-11-22 16:34:26 + 12) #define PVR_DRIVER_DATE "20230904"
> c87e859cdeb5d drivers/gpu/drm/imx/lcdc/imx-lcdc.c 361 (Marian Cichy 
> 2023-03-06 12:52:49 +0100 353).date = "20200716",
> eb92830cdbc23 drivers/gpu/drm/kmb/kmb_drv.h 19 (Edmund Dea 2020-08-26 
> 13:17:29 -0700 19) #define DRIVER_DATE  "20210223"
> f39db26c54281 drivers/gpu/drm/loongson/lsdc_drv.c 28 (Sui Jingfeng 2023-06-15 
> 22:36:12 +0800 28) #define DRIVER_DATE "20220701"
> 5fc537bfd0003 drivers/gpu/drm/mcde/mcde_drv.c 247 (Linus Walleij 2019-05-24 
> 11:20:19 +0200 210)   .date = "20180529",
> 119f5173628aa drivers/gpu/drm/mediatek/mtk_drm_drv.c 36 (CK Hu 2016-01-04 
> 18:36:34 +0100 34) #define DRIVER_DATE "20150513"
> 414c453106255 drivers/gpu/drm/mgag200/mgag200_drv.h 34 (Dave Airlie 
> 2012-04-17 15:01:25 +0100 31) #define DRIVER_DATE "20110418"
> 77145f1cbdf8d drivers/gpu/drm/nouveau/nouveau_drm.h 9 (Ben Skeggs 2012-07-31 
> 16:16:21 +1000 10) #define DRIVER_DATE   "20120801"
> cd5351f4d2b1b drivers/staging/omapdrm/omap_drv.c 27 (Rob Clark 2011-11-12 
> 12:09:40 -0600 31) #define DRIVER_DATE  "20110917"
> 4bdca11507928 drivers/gpu/drm/panthor/panthor_drv.c 1371 (Boris Brezillon 
> 2024-02-29 17:22:25 +0100 1386) .date = "20230801",
> bed41005e6174 drivers/gpu/drm/pl111/pl111_drv.c 157 (Tom Cooksey 2017-04-12 
> 20:17:46 -0700 222)   .date = "20170317",
> f64122c1f6ade drivers/gpu/drm/qxl/qxl_drv.h 52 (Dave Airlie 2013-02-25 
> 14:47:55 +1000 57) #define DRIVER_DATE "20120117"
> c0beb2a723d69 drivers/char/drm/radeon_drv.h 41 (Dave Airlie 2008-05-28 
> 13:52:28 +1000 46) #define DRIVER_DATE "20080528"
> 2048e3286f347 drivers/gpu/drm/rockchip/rockchip_drm_drv.c 34 (Mark Yao 
> 2014-08-22 18:36:26 +0800 41) #define DRIVER_DATE  "20140818"
> a61732e808672 drivers/gpu/drm/solomon/ssd130x.c 38 (Javier 

Re: [PATCH] drm: deprecate driver date

2024-04-30 Thread Daniel Vetter
On Tue, Apr 30, 2024 at 04:38:55PM +0300, Jani Nikula wrote:
> On Tue, 30 Apr 2024, Daniel Vetter  wrote:
> > Might also be a good idea to wait a bit in case there's any regression
> > reports for really old userspace. But I guess there's not a high chance
> > for that to happen here, so imo fine to just go ahead right away.
> 
> This small bit is definitely easier to revert if needed than the whole
> shebang.

So the reason I'm not super worried is that most likely it's going to be a
an old driver for a specific .ko that falls over. So as long as you split
it up per-driver it should still be a fairly minimal revert. But entirely
up to you.
-Sima
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


Re: [PATCH] drm: deprecate driver date

2024-04-30 Thread Jani Nikula
On Tue, 30 Apr 2024, Daniel Vetter  wrote:
> Might also be a good idea to wait a bit in case there's any regression
> reports for really old userspace. But I guess there's not a high chance
> for that to happen here, so imo fine to just go ahead right away.

This small bit is definitely easier to revert if needed than the whole
shebang.

BR,
Jani.


-- 
Jani Nikula, Intel


Re: [PATCH] drm: deprecate driver date

2024-04-30 Thread Daniel Vetter
On Mon, Apr 29, 2024 at 08:53:15PM +0300, Jani Nikula wrote:
> On Mon, 29 Apr 2024, Hamza Mahfooz  wrote:
> > On 4/29/24 12:43, Jani Nikula wrote:
> >> The driver date serves no useful purpose, because it's hardly ever
> >> updated. The information is misleading at best.
> >> 
> >> As described in Documentation/gpu/drm-internals.rst:
> >> 
> >>The driver date, formatted as MMDD, is meant to identify the date
> >>of the latest modification to the driver. However, as most drivers
> >>fail to update it, its value is mostly useless. The DRM core prints it
> >>to the kernel log at initialization time and passes it to userspace
> >>through the DRM_IOCTL_VERSION ioctl.
> >> 
> >> Stop printing the driver date at init, and start returning the empty
> >> string "" as driver date through the DRM_IOCTL_VERSION ioctl.
> >> 
> >> The driver date initialization in drivers and the struct drm_driver date
> >> member can be removed in follow-up.
> >> 
> >> Signed-off-by: Jani Nikula 
> >
> > I would prefer if it was dropped entirely in this patch, but if you feel
> > that would require too much back and forth, I'm okay with what is
> > currently proposed.
> 
> I can if that's what people prefer, but decided to start with this for
> the inevitable discussion before putting in the effort. ;)

Might also be a good idea to wait a bit in case there's any regression
reports for really old userspace. But I guess there's not a high chance
for that to happen here, so imo fine to just go ahead right away.
-Sima

> 
> > Reviewed-by: Hamza Mahfooz 
> 
> Thanks,
> Jani.
> 
> 
> -- 
> Jani Nikula, Intel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch


Re: [PATCH] drm: deprecate driver date

2024-04-30 Thread Jani Nikula
On Tue, 30 Apr 2024, Simon Ser  wrote:
> On Monday, April 29th, 2024 at 18:43, Jani Nikula  
> wrote:
>
>> The driver date serves no useful purpose, because it's hardly ever
>> updated. The information is misleading at best.
>> 
>> As described in Documentation/gpu/drm-internals.rst:
>> 
>> The driver date, formatted as MMDD, is meant to identify the date
>> of the latest modification to the driver. However, as most drivers
>> fail to update it, its value is mostly useless. The DRM core prints it
>> to the kernel log at initialization time and passes it to userspace
>> through the DRM_IOCTL_VERSION ioctl.
>> 
>> Stop printing the driver date at init, and start returning the empty
>> string "" as driver date through the DRM_IOCTL_VERSION ioctl.
>
> Sounds good to me.
>
> Acked-by: Simon Ser 

Thanks!

> BTW, I wonder if the driver version number (major/minor/patch) is useful?
> Do drivers update it?

I think most things these days should depend on capabilities rather than
versions.

i915 is at 1.6.0 and the last change was commit 2228ed67223f ("drm: i915
updates"). 18 years ago. From that perspective, I'd be happy to drop
them too.

However, amdgpu is at 3.57.0, with an elaborate changelog in
amdgpu_drv.c, and the last change was commit 91963397c49a ("drm/amdgpu:
Enable tunneling on high-priority compute queues"). Less than six months
ago.

I wonder if we could stop initializing and printing the version for
drivers that don't care, and leave it for drivers that do? Obviously
feels more risky than the date.

BR,
Jani.


-- 
Jani Nikula, Intel


Re: [PATCH] drm: deprecate driver date

2024-04-30 Thread Javier Martinez Canillas
Jani Nikula  writes:

> The driver date serves no useful purpose, because it's hardly ever
> updated. The information is misleading at best.
>
> As described in Documentation/gpu/drm-internals.rst:
>
>   The driver date, formatted as MMDD, is meant to identify the date
>   of the latest modification to the driver. However, as most drivers
>   fail to update it, its value is mostly useless. The DRM core prints it
>   to the kernel log at initialization time and passes it to userspace
>   through the DRM_IOCTL_VERSION ioctl.
>
> Stop printing the driver date at init, and start returning the empty
> string "" as driver date through the DRM_IOCTL_VERSION ioctl.
>
> The driver date initialization in drivers and the struct drm_driver date
> member can be removed in follow-up.
>
> Signed-off-by: Jani Nikula 
>
> ---

I never understood the value of it and so this patch makes sense to me.

Reviewed-by: Javier Martinez Canillas 

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat



Re: [PATCH] drm: deprecate driver date

2024-04-30 Thread Simon Ser
On Monday, April 29th, 2024 at 18:43, Jani Nikula  wrote:

> The driver date serves no useful purpose, because it's hardly ever
> updated. The information is misleading at best.
> 
> As described in Documentation/gpu/drm-internals.rst:
> 
> The driver date, formatted as MMDD, is meant to identify the date
> of the latest modification to the driver. However, as most drivers
> fail to update it, its value is mostly useless. The DRM core prints it
> to the kernel log at initialization time and passes it to userspace
> through the DRM_IOCTL_VERSION ioctl.
> 
> Stop printing the driver date at init, and start returning the empty
> string "" as driver date through the DRM_IOCTL_VERSION ioctl.

Sounds good to me.

Acked-by: Simon Ser 

BTW, I wonder if the driver version number (major/minor/patch) is useful?
Do drivers update it?


Re: [PATCH] drm: deprecate driver date

2024-04-29 Thread Jani Nikula
On Mon, 29 Apr 2024, Hamza Mahfooz  wrote:
> On 4/29/24 12:43, Jani Nikula wrote:
>> The driver date serves no useful purpose, because it's hardly ever
>> updated. The information is misleading at best.
>> 
>> As described in Documentation/gpu/drm-internals.rst:
>> 
>>The driver date, formatted as MMDD, is meant to identify the date
>>of the latest modification to the driver. However, as most drivers
>>fail to update it, its value is mostly useless. The DRM core prints it
>>to the kernel log at initialization time and passes it to userspace
>>through the DRM_IOCTL_VERSION ioctl.
>> 
>> Stop printing the driver date at init, and start returning the empty
>> string "" as driver date through the DRM_IOCTL_VERSION ioctl.
>> 
>> The driver date initialization in drivers and the struct drm_driver date
>> member can be removed in follow-up.
>> 
>> Signed-off-by: Jani Nikula 
>
> I would prefer if it was dropped entirely in this patch, but if you feel
> that would require too much back and forth, I'm okay with what is
> currently proposed.

I can if that's what people prefer, but decided to start with this for
the inevitable discussion before putting in the effort. ;)

> Reviewed-by: Hamza Mahfooz 

Thanks,
Jani.


-- 
Jani Nikula, Intel


Re: [PATCH] drm: deprecate driver date

2024-04-29 Thread Hamza Mahfooz

On 4/29/24 12:43, Jani Nikula wrote:

The driver date serves no useful purpose, because it's hardly ever
updated. The information is misleading at best.

As described in Documentation/gpu/drm-internals.rst:

   The driver date, formatted as MMDD, is meant to identify the date
   of the latest modification to the driver. However, as most drivers
   fail to update it, its value is mostly useless. The DRM core prints it
   to the kernel log at initialization time and passes it to userspace
   through the DRM_IOCTL_VERSION ioctl.

Stop printing the driver date at init, and start returning the empty
string "" as driver date through the DRM_IOCTL_VERSION ioctl.

The driver date initialization in drivers and the struct drm_driver date
member can be removed in follow-up.

Signed-off-by: Jani Nikula 


I would prefer if it was dropped entirely in this patch, but if you feel
that would require too much back and forth, I'm okay with what is
currently proposed.

Reviewed-by: Hamza Mahfooz 



---

The below approximates when each driver's date was last updated.

$ git grepblame "\(\.date = \".*\"\|#define.*DRIVER_DATE\)" -- drivers/gpu 
drivers/accel
fe77368c0f3e0 drivers/accel/habanalabs/common/habanalabs_drv.c 94 (Tomer Tayar 2023-02-19 
11:58:46 +0200 104)   .date = "20190505",
35b137630f08d drivers/accel/ivpu/ivpu_drv.h 20 (Jacek Lawrynowicz 2023-01-17 10:27:17 
+0100 24) #define DRIVER_DATE "20230117"
d38ceaf99ed01 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.h 43 (Alex Deucher 2015-04-20 
16:55:21 -0400 43) #define DRIVER_DATE"20150101"
61f1c4a8ab757 drivers/gpu/drm/arm/display/komeda/komeda_kms.c 51 (james qian wang (Arm 
Technology China) 2019-01-03 11:41:30 + 64)  .date = "20181101",
8e22d79240d95 drivers/gpu/drm/arm/hdlcd_drv.c 343 (Liviu Dudau 2015-04-02 19:48:39 +0100 
234)   .date = "20151021",
ad49f8602fe88 drivers/gpu/drm/arm/malidp_drv.c 232 (Liviu Dudau 2016-03-07 10:00:53 + 
571)  .date = "20160106",
4f2a8f5898ecd drivers/gpu/drm/aspeed/aspeed_gfx_drv.c 208 (Joel Stanley 2019-04-03 
10:49:08 +1030 253)  .date = "20180319",
312fec1405dd5 drivers/gpu/drm/ast/ast_drv.h 46 (Dave Airlie 2012-02-29 13:40:04 + 46) 
#define DRIVER_DATE   "20120228"
1a396789f65a2 drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c 504 (Boris Brezillon 
2015-01-06 11:13:28 +0100 741)  .date = "20141504",
9913f74fe1570 drivers/gpu/drm/exynos/exynos_drm_drv.c 37 (Marek Szyprowski 2018-05-10 
08:46:36 +0900 37) #define DRIVER_DATE"20180330"
f90cd811ae7a3 drivers/gpu/drm/gma500/psb_drv.h 43 (Arthur Borsboom 2014-03-15 22:12:17 
+0100 29) #define DRIVER_DATE "20140314"
1053d01864937 drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c 1070 (Xu YiPing 2019-08-20 
23:06:19 + 930).date = "20150718",
76c56a5affeba drivers/gpu/drm/hyperv/hyperv_drm_drv.c 22 (Deepak Rawat 2021-05-27 
04:22:28 -0700 22) #define DRIVER_DATE "2020"
3570bd989acc6 drivers/gpu/drm/i915/i915_driver.h 18 (Jani Nikula 2023-09-29 12:43:23 
+0300 18) #define DRIVER_DATE  "20230929"
4babef0708656 drivers/gpu/drm/imagination/pvr_drv.h 12 (Sarah Walker 2023-11-22 16:34:26 
+ 12) #define PVR_DRIVER_DATE "20230904"
c87e859cdeb5d drivers/gpu/drm/imx/lcdc/imx-lcdc.c 361 (Marian Cichy 2023-03-06 12:52:49 
+0100 353)  .date = "20200716",
eb92830cdbc23 drivers/gpu/drm/kmb/kmb_drv.h 19 (Edmund Dea 2020-08-26 13:17:29 -0700 19) 
#define DRIVER_DATE"20210223"
f39db26c54281 drivers/gpu/drm/loongson/lsdc_drv.c 28 (Sui Jingfeng 2023-06-15 22:36:12 
+0800 28) #define DRIVER_DATE "20220701"
5fc537bfd0003 drivers/gpu/drm/mcde/mcde_drv.c 247 (Linus Walleij 2019-05-24 11:20:19 
+0200 210) .date = "20180529",
119f5173628aa drivers/gpu/drm/mediatek/mtk_drm_drv.c 36 (CK Hu 2016-01-04 18:36:34 +0100 
34) #define DRIVER_DATE "20150513"
414c453106255 drivers/gpu/drm/mgag200/mgag200_drv.h 34 (Dave Airlie 2012-04-17 15:01:25 
+0100 31) #define DRIVER_DATE   "20110418"
77145f1cbdf8d drivers/gpu/drm/nouveau/nouveau_drm.h 9 (Ben Skeggs 2012-07-31 16:16:21 
+1000 10) #define DRIVER_DATE "20120801"
cd5351f4d2b1b drivers/staging/omapdrm/omap_drv.c 27 (Rob Clark 2011-11-12 12:09:40 -0600 
31) #define DRIVER_DATE"20110917"
4bdca11507928 drivers/gpu/drm/panthor/panthor_drv.c 1371 (Boris Brezillon 2024-02-29 
17:22:25 +0100 1386)   .date = "20230801",
bed41005e6174 drivers/gpu/drm/pl111/pl111_drv.c 157 (Tom Cooksey 2017-04-12 20:17:46 
-0700 222) .date = "20170317",
f64122c1f6ade drivers/gpu/drm/qxl/qxl_drv.h 52 (Dave Airlie 2013-02-25 14:47:55 +1000 57) 
#define DRIVER_DATE   "20120117"
c0beb2a723d69 drivers/char/drm/radeon_drv.h 41 (Dave Airlie 2008-05-28 13:52:28 +1000 46) 
#define DRIVER_DATE   "20080528"
2048e3286f347 drivers/gpu/drm/rockchip/rockchip_drm_drv.c 34 (Mark Yao 2014-08-22 
18:36:26 +0800 41) #define DRIVER_DATE"20140818"
a61732e808672 drivers/gpu/drm/solomon/ssd130x.c 38 (Javier Martinez Canillas 2022-02-14 
14:37:07