From: Farah Kassabri <fkassabri@habana.ai>
To: "De Marchi, Lucas" <lucas.demarchi@intel.com>,
Jani Nikula <jani.nikula@linux.intel.com>
Cc: "intel-xe@lists.freedesktop.org" <intel-xe@lists.freedesktop.org>,
Ohad Sharabi <osharabi@habana.ai>,
"dri-devel@lists.freedesktop.org"
<dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH 1/2] drm: print top commit sha after loading the driver
Date: Wed, 8 May 2024 11:01:43 +0000 [thread overview]
Message-ID: <6a605dbe-53c9-416b-9bdc-728c3e155256@habana.ai> (raw)
In-Reply-To: <45wxpwjv6fqzbnsivojrr2dbspnftinois7um3rrtku5j47le4@lobf5qyr2f2n>
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@habana.ai> 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@habana.ai>
>>> ---
>>> 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
next prev parent reply other threads:[~2024-05-08 12:00 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20240424100706.269523-1-fkassabri@habana.ai>
[not found] ` <87bk5s4ekb.fsf@intel.com>
2024-04-29 15:32 ` [PATCH 1/2] drm: print top commit sha after loading the driver Lucas De Marchi
2024-05-08 11:01 ` Farah Kassabri [this message]
2024-05-08 11:34 ` Jani Nikula
2024-05-16 14:17 ` Farah Kassabri
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6a605dbe-53c9-416b-9bdc-728c3e155256@habana.ai \
--to=fkassabri@habana.ai \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-xe@lists.freedesktop.org \
--cc=jani.nikula@linux.intel.com \
--cc=lucas.demarchi@intel.com \
--cc=osharabi@habana.ai \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).