Re: [PATCH 1/2] drm: print top commit sha after loading the driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux