* [PATCH] drm/panthor: Display FW version information
@ 2024-09-05 15:51 Steven Price
2024-09-05 16:45 ` Boris Brezillon
0 siblings, 1 reply; 4+ messages in thread
From: Steven Price @ 2024-09-05 15:51 UTC (permalink / raw)
To: Boris Brezillon, Liviu Dudau; +Cc: Steven Price, dri-devel, Christopher Healy
The firmware binary has a git SHA embedded into it which can be used to
identify which firmware binary is being loaded. Output this as a
drm_info() so that it's obvious from a dmesg log which firmware binary
is being used.
Signed-off-by: Steven Price <steven.price@arm.com>
---
drivers/gpu/drm/panthor/panthor_fw.c | 55 ++++++++++++++++++++++++++++
1 file changed, 55 insertions(+)
diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
index 857f3f11258a..ef007287575c 100644
--- a/drivers/gpu/drm/panthor/panthor_fw.c
+++ b/drivers/gpu/drm/panthor/panthor_fw.c
@@ -78,6 +78,12 @@ enum panthor_fw_binary_entry_type {
/** @CSF_FW_BINARY_ENTRY_TYPE_TIMELINE_METADATA: Timeline metadata interface. */
CSF_FW_BINARY_ENTRY_TYPE_TIMELINE_METADATA = 4,
+
+ /**
+ * @CSF_FW_BINARY_ENTRY_TYPE_BUILD_INFO_METADATA: Metadata about how
+ * the FW binary was built.
+ */
+ CSF_FW_BINARY_ENTRY_TYPE_BUILD_INFO_METADATA = 6
};
#define CSF_FW_BINARY_ENTRY_TYPE(ehdr) ((ehdr) & 0xff)
@@ -132,6 +138,13 @@ struct panthor_fw_binary_section_entry_hdr {
} data;
};
+struct panthor_fw_build_info_hdr {
+ /** @meta_start: Offset of the build info data in the FW binary */
+ u32 meta_start;
+ /** @meta_size: Size of the build info data in the FW binary */
+ u32 meta_size;
+};
+
/**
* struct panthor_fw_binary_iter - Firmware binary iterator
*
@@ -628,6 +641,46 @@ static int panthor_fw_load_section_entry(struct panthor_device *ptdev,
return 0;
}
+static int panthor_fw_read_build_info(struct panthor_device *ptdev,
+ const struct firmware *fw,
+ struct panthor_fw_binary_iter *iter,
+ u32 ehdr)
+{
+ struct panthor_fw_build_info_hdr hdr;
+ char header[9];
+ const char git_sha_header[sizeof(header)] = "git_sha: ";
+ int ret;
+
+ ret = panthor_fw_binary_iter_read(ptdev, iter, &hdr, sizeof(hdr));
+ if (ret)
+ return ret;
+
+ if (hdr.meta_start > fw->size ||
+ hdr.meta_start + hdr.meta_size > fw->size) {
+ drm_err(&ptdev->base, "Firmware build info corrupt\n");
+ /* We don't need the build info, so continue */
+ return 0;
+ }
+
+ if (memcmp(git_sha_header, fw->data + hdr.meta_start,
+ sizeof(git_sha_header))) {
+ /* Not the expected header, this isn't metadata we understand */
+ return 0;
+ }
+
+ /* Check that the git SHA is NULL terminated as expected */
+ if (fw->data[hdr.meta_start + hdr.meta_size - 1] != '\0') {
+ drm_warn(&ptdev->base, "Firmware's git sha is not NULL terminated\n");
+ /* Don't treat as fatal */
+ return 0;
+ }
+
+ drm_info(&ptdev->base, "Firmware git sha: %s\n",
+ fw->data + hdr.meta_start + sizeof(git_sha_header));
+
+ return 0;
+}
+
static void
panthor_reload_fw_sections(struct panthor_device *ptdev, bool full_reload)
{
@@ -672,6 +725,8 @@ static int panthor_fw_load_entry(struct panthor_device *ptdev,
switch (CSF_FW_BINARY_ENTRY_TYPE(ehdr)) {
case CSF_FW_BINARY_ENTRY_TYPE_IFACE:
return panthor_fw_load_section_entry(ptdev, fw, &eiter, ehdr);
+ case CSF_FW_BINARY_ENTRY_TYPE_BUILD_INFO_METADATA:
+ return panthor_fw_read_build_info(ptdev, fw, &eiter, ehdr);
/* FIXME: handle those entry types? */
case CSF_FW_BINARY_ENTRY_TYPE_CONFIG:
--
2.39.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/panthor: Display FW version information
2024-09-05 15:51 [PATCH] drm/panthor: Display FW version information Steven Price
@ 2024-09-05 16:45 ` Boris Brezillon
2024-09-05 21:08 ` Liviu Dudau
0 siblings, 1 reply; 4+ messages in thread
From: Boris Brezillon @ 2024-09-05 16:45 UTC (permalink / raw)
To: Steven Price; +Cc: Liviu Dudau, dri-devel, Christopher Healy
On Thu, 5 Sep 2024 16:51:44 +0100
Steven Price <steven.price@arm.com> wrote:
> The firmware binary has a git SHA embedded into it which can be used to
> identify which firmware binary is being loaded. Output this as a
> drm_info() so that it's obvious from a dmesg log which firmware binary
> is being used.
>
> Signed-off-by: Steven Price <steven.price@arm.com>
Just one formatting issue mentioned below, looks good otherwise.
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/gpu/drm/panthor/panthor_fw.c | 55 ++++++++++++++++++++++++++++
> 1 file changed, 55 insertions(+)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
> index 857f3f11258a..ef007287575c 100644
> --- a/drivers/gpu/drm/panthor/panthor_fw.c
> +++ b/drivers/gpu/drm/panthor/panthor_fw.c
> @@ -78,6 +78,12 @@ enum panthor_fw_binary_entry_type {
>
> /** @CSF_FW_BINARY_ENTRY_TYPE_TIMELINE_METADATA: Timeline metadata interface. */
> CSF_FW_BINARY_ENTRY_TYPE_TIMELINE_METADATA = 4,
> +
> + /**
> + * @CSF_FW_BINARY_ENTRY_TYPE_BUILD_INFO_METADATA: Metadata about how
> + * the FW binary was built.
> + */
> + CSF_FW_BINARY_ENTRY_TYPE_BUILD_INFO_METADATA = 6
> };
>
> #define CSF_FW_BINARY_ENTRY_TYPE(ehdr) ((ehdr) & 0xff)
> @@ -132,6 +138,13 @@ struct panthor_fw_binary_section_entry_hdr {
> } data;
> };
>
> +struct panthor_fw_build_info_hdr {
> + /** @meta_start: Offset of the build info data in the FW binary */
> + u32 meta_start;
> + /** @meta_size: Size of the build info data in the FW binary */
> + u32 meta_size;
> +};
> +
> /**
> * struct panthor_fw_binary_iter - Firmware binary iterator
> *
> @@ -628,6 +641,46 @@ static int panthor_fw_load_section_entry(struct panthor_device *ptdev,
> return 0;
> }
>
> +static int panthor_fw_read_build_info(struct panthor_device *ptdev,
> + const struct firmware *fw,
> + struct panthor_fw_binary_iter *iter,
> + u32 ehdr)
> +{
> + struct panthor_fw_build_info_hdr hdr;
> + char header[9];
> + const char git_sha_header[sizeof(header)] = "git_sha: ";
> + int ret;
> +
> + ret = panthor_fw_binary_iter_read(ptdev, iter, &hdr, sizeof(hdr));
> + if (ret)
> + return ret;
> +
> + if (hdr.meta_start > fw->size ||
> + hdr.meta_start + hdr.meta_size > fw->size) {
> + drm_err(&ptdev->base, "Firmware build info corrupt\n");
> + /* We don't need the build info, so continue */
> + return 0;
> + }
> +
> + if (memcmp(git_sha_header, fw->data + hdr.meta_start,
> + sizeof(git_sha_header))) {
Indentation seems broken here:
if (memcmp(git_sha_header, fw->data + hdr.meta_start,
sizeof(git_sha_header))) {
> + /* Not the expected header, this isn't metadata we understand */
> + return 0;
> + }
> +
> + /* Check that the git SHA is NULL terminated as expected */
> + if (fw->data[hdr.meta_start + hdr.meta_size - 1] != '\0') {
> + drm_warn(&ptdev->base, "Firmware's git sha is not NULL terminated\n");
> + /* Don't treat as fatal */
> + return 0;
> + }
> +
> + drm_info(&ptdev->base, "Firmware git sha: %s\n",
> + fw->data + hdr.meta_start + sizeof(git_sha_header));
Maybe we should also change the "FW vX.Y.Z" message into "FW interface
vX.Y.Z" to clarify things.
> +
> + return 0;
> +}
> +
> static void
> panthor_reload_fw_sections(struct panthor_device *ptdev, bool full_reload)
> {
> @@ -672,6 +725,8 @@ static int panthor_fw_load_entry(struct panthor_device *ptdev,
> switch (CSF_FW_BINARY_ENTRY_TYPE(ehdr)) {
> case CSF_FW_BINARY_ENTRY_TYPE_IFACE:
> return panthor_fw_load_section_entry(ptdev, fw, &eiter, ehdr);
> + case CSF_FW_BINARY_ENTRY_TYPE_BUILD_INFO_METADATA:
> + return panthor_fw_read_build_info(ptdev, fw, &eiter, ehdr);
>
> /* FIXME: handle those entry types? */
> case CSF_FW_BINARY_ENTRY_TYPE_CONFIG:
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/panthor: Display FW version information
2024-09-05 16:45 ` Boris Brezillon
@ 2024-09-05 21:08 ` Liviu Dudau
2024-09-09 13:58 ` Carsten Haitzler
0 siblings, 1 reply; 4+ messages in thread
From: Liviu Dudau @ 2024-09-05 21:08 UTC (permalink / raw)
To: Boris Brezillon; +Cc: Steven Price, dri-devel, Christopher Healy
On Thu, Sep 05, 2024 at 06:45:28PM +0200, Boris Brezillon wrote:
> On Thu, 5 Sep 2024 16:51:44 +0100
> Steven Price <steven.price@arm.com> wrote:
>
> > The firmware binary has a git SHA embedded into it which can be used to
> > identify which firmware binary is being loaded. Output this as a
> > drm_info() so that it's obvious from a dmesg log which firmware binary
> > is being used.
> >
> > Signed-off-by: Steven Price <steven.price@arm.com>
>
> Just one formatting issue mentioned below, looks good otherwise.
>
> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
>
> > ---
> > drivers/gpu/drm/panthor/panthor_fw.c | 55 ++++++++++++++++++++++++++++
> > 1 file changed, 55 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
> > index 857f3f11258a..ef007287575c 100644
> > --- a/drivers/gpu/drm/panthor/panthor_fw.c
> > +++ b/drivers/gpu/drm/panthor/panthor_fw.c
> > @@ -78,6 +78,12 @@ enum panthor_fw_binary_entry_type {
> >
> > /** @CSF_FW_BINARY_ENTRY_TYPE_TIMELINE_METADATA: Timeline metadata interface. */
> > CSF_FW_BINARY_ENTRY_TYPE_TIMELINE_METADATA = 4,
> > +
> > + /**
> > + * @CSF_FW_BINARY_ENTRY_TYPE_BUILD_INFO_METADATA: Metadata about how
> > + * the FW binary was built.
> > + */
> > + CSF_FW_BINARY_ENTRY_TYPE_BUILD_INFO_METADATA = 6
> > };
> >
> > #define CSF_FW_BINARY_ENTRY_TYPE(ehdr) ((ehdr) & 0xff)
> > @@ -132,6 +138,13 @@ struct panthor_fw_binary_section_entry_hdr {
> > } data;
> > };
> >
> > +struct panthor_fw_build_info_hdr {
> > + /** @meta_start: Offset of the build info data in the FW binary */
> > + u32 meta_start;
> > + /** @meta_size: Size of the build info data in the FW binary */
> > + u32 meta_size;
> > +};
> > +
> > /**
> > * struct panthor_fw_binary_iter - Firmware binary iterator
> > *
> > @@ -628,6 +641,46 @@ static int panthor_fw_load_section_entry(struct panthor_device *ptdev,
> > return 0;
> > }
> >
> > +static int panthor_fw_read_build_info(struct panthor_device *ptdev,
> > + const struct firmware *fw,
> > + struct panthor_fw_binary_iter *iter,
> > + u32 ehdr)
> > +{
> > + struct panthor_fw_build_info_hdr hdr;
> > + char header[9];
> > + const char git_sha_header[sizeof(header)] = "git_sha: ";
> > + int ret;
> > +
> > + ret = panthor_fw_binary_iter_read(ptdev, iter, &hdr, sizeof(hdr));
> > + if (ret)
> > + return ret;
> > +
> > + if (hdr.meta_start > fw->size ||
> > + hdr.meta_start + hdr.meta_size > fw->size) {
> > + drm_err(&ptdev->base, "Firmware build info corrupt\n");
> > + /* We don't need the build info, so continue */
> > + return 0;
> > + }
> > +
> > + if (memcmp(git_sha_header, fw->data + hdr.meta_start,
> > + sizeof(git_sha_header))) {
>
> Indentation seems broken here:
>
> if (memcmp(git_sha_header, fw->data + hdr.meta_start,
> sizeof(git_sha_header))) {
>
> > + /* Not the expected header, this isn't metadata we understand */
> > + return 0;
> > + }
> > +
> > + /* Check that the git SHA is NULL terminated as expected */
> > + if (fw->data[hdr.meta_start + hdr.meta_size - 1] != '\0') {
> > + drm_warn(&ptdev->base, "Firmware's git sha is not NULL terminated\n");
> > + /* Don't treat as fatal */
> > + return 0;
> > + }
> > +
> > + drm_info(&ptdev->base, "Firmware git sha: %s\n",
> > + fw->data + hdr.meta_start + sizeof(git_sha_header));
>
> Maybe we should also change the "FW vX.Y.Z" message into "FW interface
> vX.Y.Z" to clarify things.
Or something like "FW using interface vX.Y.Z"
Best regards,
Liviu
>
> > +
> > + return 0;
> > +}
> > +
> > static void
> > panthor_reload_fw_sections(struct panthor_device *ptdev, bool full_reload)
> > {
> > @@ -672,6 +725,8 @@ static int panthor_fw_load_entry(struct panthor_device *ptdev,
> > switch (CSF_FW_BINARY_ENTRY_TYPE(ehdr)) {
> > case CSF_FW_BINARY_ENTRY_TYPE_IFACE:
> > return panthor_fw_load_section_entry(ptdev, fw, &eiter, ehdr);
> > + case CSF_FW_BINARY_ENTRY_TYPE_BUILD_INFO_METADATA:
> > + return panthor_fw_read_build_info(ptdev, fw, &eiter, ehdr);
> >
> > /* FIXME: handle those entry types? */
> > case CSF_FW_BINARY_ENTRY_TYPE_CONFIG:
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/panthor: Display FW version information
2024-09-05 21:08 ` Liviu Dudau
@ 2024-09-09 13:58 ` Carsten Haitzler
0 siblings, 0 replies; 4+ messages in thread
From: Carsten Haitzler @ 2024-09-09 13:58 UTC (permalink / raw)
To: dri-devel
On 9/5/24 10:08 PM, Liviu Dudau wrote:
> On Thu, Sep 05, 2024 at 06:45:28PM +0200, Boris Brezillon wrote:
>> On Thu, 5 Sep 2024 16:51:44 +0100
>> Steven Price <steven.price@arm.com> wrote:
>>
>>> The firmware binary has a git SHA embedded into it which can be used to
>>> identify which firmware binary is being loaded. Output this as a
>>> drm_info() so that it's obvious from a dmesg log which firmware binary
>>> is being used.
>>>
>>> Signed-off-by: Steven Price <steven.price@arm.com>
>>
>> Just one formatting issue mentioned below, looks good otherwise.
>>
>> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
>
> Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
>
>>
>>> ---
>>> drivers/gpu/drm/panthor/panthor_fw.c | 55 ++++++++++++++++++++++++++++
>>> 1 file changed, 55 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
>>> index 857f3f11258a..ef007287575c 100644
>>> --- a/drivers/gpu/drm/panthor/panthor_fw.c
>>> +++ b/drivers/gpu/drm/panthor/panthor_fw.c
>>> @@ -78,6 +78,12 @@ enum panthor_fw_binary_entry_type {
>>>
>>> /** @CSF_FW_BINARY_ENTRY_TYPE_TIMELINE_METADATA: Timeline metadata interface. */
>>> CSF_FW_BINARY_ENTRY_TYPE_TIMELINE_METADATA = 4,
>>> +
>>> + /**
>>> + * @CSF_FW_BINARY_ENTRY_TYPE_BUILD_INFO_METADATA: Metadata about how
>>> + * the FW binary was built.
>>> + */
>>> + CSF_FW_BINARY_ENTRY_TYPE_BUILD_INFO_METADATA = 6
>>> };
>>>
>>> #define CSF_FW_BINARY_ENTRY_TYPE(ehdr) ((ehdr) & 0xff)
>>> @@ -132,6 +138,13 @@ struct panthor_fw_binary_section_entry_hdr {
>>> } data;
>>> };
>>>
>>> +struct panthor_fw_build_info_hdr {
>>> + /** @meta_start: Offset of the build info data in the FW binary */
>>> + u32 meta_start;
>>> + /** @meta_size: Size of the build info data in the FW binary */
>>> + u32 meta_size;
>>> +};
>>> +
>>> /**
>>> * struct panthor_fw_binary_iter - Firmware binary iterator
>>> *
>>> @@ -628,6 +641,46 @@ static int panthor_fw_load_section_entry(struct panthor_device *ptdev,
>>> return 0;
>>> }
>>>
>>> +static int panthor_fw_read_build_info(struct panthor_device *ptdev,
>>> + const struct firmware *fw,
>>> + struct panthor_fw_binary_iter *iter,
>>> + u32 ehdr)
>>> +{
>>> + struct panthor_fw_build_info_hdr hdr;
>>> + char header[9];
>>> + const char git_sha_header[sizeof(header)] = "git_sha: ";
>>> + int ret;
>>> +
>>> + ret = panthor_fw_binary_iter_read(ptdev, iter, &hdr, sizeof(hdr));
>>> + if (ret)
>>> + return ret;
>>> +
>>> + if (hdr.meta_start > fw->size ||
>>> + hdr.meta_start + hdr.meta_size > fw->size) {
>>> + drm_err(&ptdev->base, "Firmware build info corrupt\n");
>>> + /* We don't need the build info, so continue */
>>> + return 0;
>>> + }
>>> +
>>> + if (memcmp(git_sha_header, fw->data + hdr.meta_start,
>>> + sizeof(git_sha_header))) {
>>
>> Indentation seems broken here:
>>
>> if (memcmp(git_sha_header, fw->data + hdr.meta_start,
>> sizeof(git_sha_header))) {
>>
>>> + /* Not the expected header, this isn't metadata we understand */
>>> + return 0;
>>> + }
>>> +
>>> + /* Check that the git SHA is NULL terminated as expected */
>>> + if (fw->data[hdr.meta_start + hdr.meta_size - 1] != '\0') {
>>> + drm_warn(&ptdev->base, "Firmware's git sha is not NULL terminated\n");
>>> + /* Don't treat as fatal */
>>> + return 0;
>>> + }
>>> +
>>> + drm_info(&ptdev->base, "Firmware git sha: %s\n",
>>> + fw->data + hdr.meta_start + sizeof(git_sha_header));
>>
>> Maybe we should also change the "FW vX.Y.Z" message into "FW interface
>> vX.Y.Z" to clarify things.
>
> Or something like "FW using interface vX.Y.Z"
+1. I like the idea. It's a bit confusing otherwise.
> Best regards,
> Liviu
>
>>
>>> +
>>> + return 0;
>>> +}
>>> +
>>> static void
>>> panthor_reload_fw_sections(struct panthor_device *ptdev, bool full_reload)
>>> {
>>> @@ -672,6 +725,8 @@ static int panthor_fw_load_entry(struct panthor_device *ptdev,
>>> switch (CSF_FW_BINARY_ENTRY_TYPE(ehdr)) {
>>> case CSF_FW_BINARY_ENTRY_TYPE_IFACE:
>>> return panthor_fw_load_section_entry(ptdev, fw, &eiter, ehdr);
>>> + case CSF_FW_BINARY_ENTRY_TYPE_BUILD_INFO_METADATA:
>>> + return panthor_fw_read_build_info(ptdev, fw, &eiter, ehdr);
>>>
>>> /* FIXME: handle those entry types? */
>>> case CSF_FW_BINARY_ENTRY_TYPE_CONFIG:
>>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-09-09 13:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-05 15:51 [PATCH] drm/panthor: Display FW version information Steven Price
2024-09-05 16:45 ` Boris Brezillon
2024-09-05 21:08 ` Liviu Dudau
2024-09-09 13:58 ` Carsten Haitzler
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.