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
