On 4/29/2024 18:32, Lucas De Marchi wrote:
> On Mon, Apr 29, 2024 at 02:02:28PM GMT, Jani Nikula wrote:
>> On Wed, 24 Apr 2024, Farah Kassabri <fkassabri@xxxxxxxxx> wrote:
>>> Add the last driver sha to the existing log message
>>> which prints the drm device info.
>>
>> The commit message fails to answer the most important question: why?
>>
>> Especially, what makes drm devices such special snowflakes that they'd
>> need to be the only ones in the kernel to print git commit sha1's?
>
>
> the closest to what was added here would be srcversion:
>
> $ modinfo build64/drivers/gpu/drm/xe/xe.ko | grep srcversion
> srcversion: 0EA08A43AC399A0D942740
>
> which makes more sense and doesn't depend on the git tree checkout and
> other possible problems with dirty checkouts. If you want to print it
> on module load to be able to tell from a log, you could probably just
> access mod->srcversion.
>
> but I'm not sure what we are trying to accomplish here and the commit
> message certainly missed that. Please Cc dri-devel on changes outside xe
> and provide the motivation in the commit message.
The main reason why we need this sha, is because our driver will run in
multiple environments and setups (whether it's for clients, our labs or
data center), each setup could work with different driver build and it's
very convenient to see what is the last sha in the running driver
in order to see if some fix is inside or not for example.
The srcversion is not good enough as it's doesn't server the purpose and
it's not incrementing at the same rate as the commits sha.
Note that this apply not to all drm devices only xe, and it's optional
for other devices to set it or not, and in case it's not set this patch
will not affect the existing drm log.
you can see we're already doing that for habanalabs driver.
I'll update the commit message, and the dri-level alreay in CC in case
they see any issue with this change.
BR,
Farah
>
> thanks
> Lucas De Marchi
>
>
>>
>> BR,
>> Jani.
>>
>>>
>>> Signed-off-by: Farah Kassabri <fkassabri@xxxxxxxxx>
>>> ---
>>> drivers/gpu/drm/drm_drv.c | 6 +++---
>>> include/drm/drm_drv.h | 2 ++
>>> 2 files changed, 5 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
>>> index 535b624d4c9d..e0f7af1b6ec3 100644
>>> --- a/drivers/gpu/drm/drm_drv.c
>>> +++ b/drivers/gpu/drm/drm_drv.c
>>> @@ -947,10 +947,10 @@ int drm_dev_register(struct drm_device *dev,
>>> unsigned long flags)
>>> }
>>> drm_panic_register(dev);
>>>
>>> - DRM_INFO("Initialized %s %d.%d.%d %s for %s on minor %d\n",
>>> + DRM_INFO("Initialized %s %d.%d.%d%s %s for %s on minor %d\n",
>>> driver->name, driver->major, driver->minor,
>>> - driver->patchlevel, driver->date,
>>> - dev->dev ? dev_name(dev->dev) : "virtual device",
>>> + driver->patchlevel, driver->git_sha ? driver->git_sha : "",
>>> + driver->date, dev->dev ? dev_name(dev->dev) : "virtual
>>> device",
>>> dev->primary ? dev->primary->index : dev->accel->index);
>>>
>>> goto out_unlock;
>>> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
>>> index 8878260d7529..7578a1f4ce74 100644
>>> --- a/include/drm/drm_drv.h
>>> +++ b/include/drm/drm_drv.h
>>> @@ -407,6 +407,8 @@ struct drm_driver {
>>> int minor;
>>> /** @patchlevel: driver patch level */
>>> int patchlevel;
>>> + /** @git_sha: driver last commit sha */
>>> + char *git_sha;
>>> /** @name: driver name */
>>> char *name;
>>> /** @desc: driver description */
>>
>> --
>> Jani Nikula, Intel
