[radeon] Hardcoded DRIVER_DATE?

[Sedat Dilek]

Resend to new ML-adress <dri-devel at lists.freedesktop.org>.

- Sedat -


---------- Forwarded message ----------
From: Sedat Dilek <sedat.dilek at googlemail.com>
Date: Mon, Apr 19, 2010 at 11:30 AM
Subject: [radeon] Hardcoded DRIVER_DATE?
To: DRI <dri-devel at lists.sourceforge.net>
Cc: Dave Airlie <airlied at redhat.com>


Hi,

today I pulled drm-linus GIT branch into Linus-tree (2.6.34-rc4-git6).

Again, I was seeing a version-bump of the radeon-kms wrapper/driver,
but not in the driver-date:

[dmesg]
...
[   71.347298] [drm] Initialized radeon 2.3.0 20080528 for
0000:01:00.0 on minor 0
...

While inspecting where the driver-date is defined, I found:

$ grep DRIVER_DATE -r drivers/gpu/drm/radeon/
drivers/gpu/drm/radeon/radeon_drv.h:#define DRIVER_DATE         "20080528"
drivers/gpu/drm/radeon/radeon_drv.c:    .date = DRIVER_DATE,
drivers/gpu/drm/radeon/radeon_drv.c:    .date = DRIVER_DATE,

Looking closer into both files ([1] and [2]) reveals:

[drivers/gpu/drm/radeon/radeon_drv.h]
...
#define DRIVER_NAME             "radeon"
#define DRIVER_DESC             "ATI Radeon"
#define DRIVER_DATE             "20080528"
...
 * 1.32- fixes for rv740 setup
 * 1.33- Add r6xx/r7xx const buffer support
 */
#define DRIVER_MAJOR            1
#define DRIVER_MINOR            33
#define DRIVER_PATCHLEVEL       0
...

[drivers/gpu/drm/radeon/radeon_drv.c]
...
/*
 * KMS wrapper.
 * - 2.0.0 - initial interface
 * - 2.1.0 - add square tiling interface
 * - 2.2.0 - add r6xx/r7xx const buffer support
 * - 2.3.0 - add MSPOS + 3D texture + r500 VAP regs
 */
#define KMS_DRIVER_MAJOR        2
#define KMS_DRIVER_MINOR        3
#define KMS_DRIVER_PATCHLEVEL   0
...
       .name = DRIVER_NAME,
       .desc = DRIVER_DESC,
       .date = DRIVER_DATE,
       .major = DRIVER_MAJOR,
       .minor = DRIVER_MINOR,
       .patchlevel = DRIVER_PATCHLEVEL,
...

My first thoughts on this was, if you manage two files with version
informations, it is human to forget to bump it in both.
Dave apologized to be "lazy" for the changes on IRC.

The second thought was, why the hell are there two version-strings?
1.33.0 (H-file) and 2.3.0 (C-file).
OK, the second one is for the KMS-wrapper, the first one is for radeon
kernel-module in general.

The version-infos from the H-file/C-file can be read in dmesg:
...
[    9.861334] [drm] Initialized radeon 1.33.0 20080528 for
0000:01:00.0 on minor 0
...
[   71.347298] [drm] Initialized radeon 2.3.0 20080528 for
0000:01:00.0 on minor 0
...

Do changes in radeon_drv.c (KMS-wrapper) require also a version-bump
in the header-file?
I think yes.

IIRC this is missing and version should be bumped:

[drivers/gpu/drm/radeon/radeon_drv.h]
...
 * 1.33- Add r6xx/r7xx const buffer support
+ * 1.34- add MSPOS + 3D texture + r500 VAP regs
 */
#define DRIVER_MAJOR            1
-#define DRIVER_MINOR           33
+#define DRIVER_MINOR           34
#define DRIVER_PATCHLEVEL       0
...

My third thought was, when there is a version-string for radeon
kernel-module why don't I see them via modinfo?

$ sudo modinfo radeon | grep -i ver
filename:
/lib/modules/2.6.34-rc4-iniza-686-kms/kernel/drivers/gpu/drm/radeon/radeon.ko
srcversion:     27576A7F4ADA88E07720FD4
vermagic:       2.6.34-rc4-iniza-686-kms SMP preempt mod_unload modversions 686

Unfortunately, NO.
Is there something speaking against to have it in the module-description?
How does other kernel-module handle such things? Usual - unusual? Guideline?

By reading this report one could think it is only "cosmetics" (and not
writing so much text), but why use a driver-date that is simply wrong
or unused?

Kind Regards,
- Sedat -

References:
=======
[1] http://git.kernel.org/?p=linux/kernel/git/airlied/drm-2.6.git;a=blob_plain;f=drivers/gpu/drm/radeon/radeon_drv.h;hb=cae94b0ad9d147152af77b971a7234faf20027a9
[2] http://git.kernel.org/?p=linux/kernel/git/airlied/drm-2.6.git;a=blob_plain;f=drivers/gpu/drm/radeon/radeon_drv.c;hb=cae94b0ad9d147152af77b971a7234faf20027a9