* [PATCH] etnaviv: Restore some id values
@ 2024-02-29 19:55 Christian Gmeiner
2024-03-01 9:27 ` Tomeu Vizoso
2024-03-01 10:08 ` Lucas Stach
0 siblings, 2 replies; 4+ messages in thread
From: Christian Gmeiner @ 2024-02-29 19:55 UTC (permalink / raw)
To: Lucas Stach, Russell King, Christian Gmeiner, David Airlie,
Daniel Vetter
Cc: Christian Gmeiner, stable, etnaviv, dri-devel, linux-kernel
From: Christian Gmeiner <cgmeiner@igalia.com>
The hwdb selection logic as a feature that allows it to mark some fields
as 'don't care'. If we match with such a field we memcpy(..)
the current etnaviv_chip_identity into ident.
This step can overwrite some id values read from the GPU with the
'don't care' value.
Fix this issue by restoring the affected values after the memcpy(..).
As this is crucial for user space to know when this feature works as
expected increment the minor version too.
Fixes: 4078a1186dd3 ("drm/etnaviv: update hwdb selection logic")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Gmeiner <cgmeiner@igalia.com>
---
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 2 +-
drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 14 ++++++++++++++
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 6228ce603248..9a2965741dab 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -494,7 +494,7 @@ static const struct drm_driver etnaviv_drm_driver = {
.desc = "etnaviv DRM",
.date = "20151214",
.major = 1,
- .minor = 3,
+ .minor = 4,
};
/*
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
index 67201242438b..1e38d66702f1 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
@@ -265,6 +265,9 @@ static const struct etnaviv_chip_identity etnaviv_chip_identities[] = {
bool etnaviv_fill_identity_from_hwdb(struct etnaviv_gpu *gpu)
{
struct etnaviv_chip_identity *ident = &gpu->identity;
+ const u32 product_id = ident->product_id;
+ const u32 customer_id = ident->customer_id;
+ const u32 eco_id = ident->eco_id;
int i;
for (i = 0; i < ARRAY_SIZE(etnaviv_chip_identities); i++) {
@@ -278,6 +281,17 @@ bool etnaviv_fill_identity_from_hwdb(struct etnaviv_gpu *gpu)
etnaviv_chip_identities[i].eco_id == ~0U)) {
memcpy(ident, &etnaviv_chip_identities[i],
sizeof(*ident));
+
+ /* Restore some id values if ~0U aka 'don't care' is used. */
+ if (etnaviv_chip_identities[i].product_id == ~0U)
+ ident->product_id = product_id;
+
+ if (etnaviv_chip_identities[i].customer_id == ~0U)
+ ident->customer_id = customer_id;
+
+ if (etnaviv_chip_identities[i].eco_id == ~0U)
+ ident->eco_id = eco_id;
+
return true;
}
}
--
2.44.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] etnaviv: Restore some id values
2024-02-29 19:55 [PATCH] etnaviv: Restore some id values Christian Gmeiner
@ 2024-03-01 9:27 ` Tomeu Vizoso
2024-03-01 10:08 ` Lucas Stach
1 sibling, 0 replies; 4+ messages in thread
From: Tomeu Vizoso @ 2024-03-01 9:27 UTC (permalink / raw)
To: Christian Gmeiner
Cc: Lucas Stach, Russell King, David Airlie, Daniel Vetter,
Christian Gmeiner, stable, etnaviv, dri-devel, linux-kernel
On Thu, Feb 29, 2024 at 8:55 PM Christian Gmeiner
<christian.gmeiner@gmail.com> wrote:
>
> From: Christian Gmeiner <cgmeiner@igalia.com>
>
> The hwdb selection logic as a feature that allows it to mark some fields
> as 'don't care'. If we match with such a field we memcpy(..)
> the current etnaviv_chip_identity into ident.
>
> This step can overwrite some id values read from the GPU with the
> 'don't care' value.
>
> Fix this issue by restoring the affected values after the memcpy(..).
>
> As this is crucial for user space to know when this feature works as
> expected increment the minor version too.
>
> Fixes: 4078a1186dd3 ("drm/etnaviv: update hwdb selection logic")
> Cc: stable@vger.kernel.org
Oops.
Reviewed-by: Tomeu Vizoso <tomeu@tomeuvizoso.net>
Cheers,
Tomeu
> Signed-off-by: Christian Gmeiner <cgmeiner@igalia.com>
> ---
> drivers/gpu/drm/etnaviv/etnaviv_drv.c | 2 +-
> drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 14 ++++++++++++++
> 2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 6228ce603248..9a2965741dab 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -494,7 +494,7 @@ static const struct drm_driver etnaviv_drm_driver = {
> .desc = "etnaviv DRM",
> .date = "20151214",
> .major = 1,
> - .minor = 3,
> + .minor = 4,
> };
>
> /*
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> index 67201242438b..1e38d66702f1 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> @@ -265,6 +265,9 @@ static const struct etnaviv_chip_identity etnaviv_chip_identities[] = {
> bool etnaviv_fill_identity_from_hwdb(struct etnaviv_gpu *gpu)
> {
> struct etnaviv_chip_identity *ident = &gpu->identity;
> + const u32 product_id = ident->product_id;
> + const u32 customer_id = ident->customer_id;
> + const u32 eco_id = ident->eco_id;
> int i;
>
> for (i = 0; i < ARRAY_SIZE(etnaviv_chip_identities); i++) {
> @@ -278,6 +281,17 @@ bool etnaviv_fill_identity_from_hwdb(struct etnaviv_gpu *gpu)
> etnaviv_chip_identities[i].eco_id == ~0U)) {
> memcpy(ident, &etnaviv_chip_identities[i],
> sizeof(*ident));
> +
> + /* Restore some id values if ~0U aka 'don't care' is used. */
> + if (etnaviv_chip_identities[i].product_id == ~0U)
> + ident->product_id = product_id;
> +
> + if (etnaviv_chip_identities[i].customer_id == ~0U)
> + ident->customer_id = customer_id;
> +
> + if (etnaviv_chip_identities[i].eco_id == ~0U)
> + ident->eco_id = eco_id;
> +
> return true;
> }
> }
> --
> 2.44.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] etnaviv: Restore some id values
2024-02-29 19:55 [PATCH] etnaviv: Restore some id values Christian Gmeiner
2024-03-01 9:27 ` Tomeu Vizoso
@ 2024-03-01 10:08 ` Lucas Stach
2024-03-01 13:30 ` Christian Gmeiner
1 sibling, 1 reply; 4+ messages in thread
From: Lucas Stach @ 2024-03-01 10:08 UTC (permalink / raw)
To: Christian Gmeiner, Russell King, David Airlie, Daniel Vetter
Cc: Christian Gmeiner, stable, etnaviv, dri-devel, linux-kernel
Hi Christian,
prefix for etnaviv kernel patches should be "drm/etnaviv: ..."
Am Donnerstag, dem 29.02.2024 um 20:55 +0100 schrieb Christian Gmeiner:
> From: Christian Gmeiner <cgmeiner@igalia.com>
>
> The hwdb selection logic as a feature that allows it to mark some fields
> as 'don't care'. If we match with such a field we memcpy(..)
> the current etnaviv_chip_identity into ident.
>
> This step can overwrite some id values read from the GPU with the
> 'don't care' value.
>
> Fix this issue by restoring the affected values after the memcpy(..).
>
> As this is crucial for user space to know when this feature works as
> expected increment the minor version too.
Uh, right. Lying to userspace about the GPU identity doesn't seem like
a good idea.
>
> Fixes: 4078a1186dd3 ("drm/etnaviv: update hwdb selection logic")
> Cc: stable@vger.kernel.org
> Signed-off-by: Christian Gmeiner <cgmeiner@igalia.com>
> ---
> drivers/gpu/drm/etnaviv/etnaviv_drv.c | 2 +-
> drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 14 ++++++++++++++
> 2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 6228ce603248..9a2965741dab 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -494,7 +494,7 @@ static const struct drm_driver etnaviv_drm_driver = {
> .desc = "etnaviv DRM",
> .date = "20151214",
> .major = 1,
> - .minor = 3,
> + .minor = 4,
> };
>
> /*
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> index 67201242438b..1e38d66702f1 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> @@ -265,6 +265,9 @@ static const struct etnaviv_chip_identity etnaviv_chip_identities[] = {
> bool etnaviv_fill_identity_from_hwdb(struct etnaviv_gpu *gpu)
> {
> struct etnaviv_chip_identity *ident = &gpu->identity;
> + const u32 product_id = ident->product_id;
> + const u32 customer_id = ident->customer_id;
> + const u32 eco_id = ident->eco_id;
> int i;
>
> for (i = 0; i < ARRAY_SIZE(etnaviv_chip_identities); i++) {
> @@ -278,6 +281,17 @@ bool etnaviv_fill_identity_from_hwdb(struct etnaviv_gpu *gpu)
> etnaviv_chip_identities[i].eco_id == ~0U)) {
> memcpy(ident, &etnaviv_chip_identities[i],
> sizeof(*ident));
> +
> + /* Restore some id values if ~0U aka 'don't care' is used. */
> + if (etnaviv_chip_identities[i].product_id == ~0U)
You can drop all those if clauses. Either the hwdb value is an exact
match and the value is the same or it's the don't care value and you
want to restore it, so a simple unconditional assignment would make
this a bit more compact.
Regards,
Lucas
> + ident->product_id = product_id;
> +
> + if (etnaviv_chip_identities[i].customer_id == ~0U)
> + ident->customer_id = customer_id;
> +
> + if (etnaviv_chip_identities[i].eco_id == ~0U)
> + ident->eco_id = eco_id;
> +
> return true;
> }
> }
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] etnaviv: Restore some id values
2024-03-01 10:08 ` Lucas Stach
@ 2024-03-01 13:30 ` Christian Gmeiner
0 siblings, 0 replies; 4+ messages in thread
From: Christian Gmeiner @ 2024-03-01 13:30 UTC (permalink / raw)
To: Lucas Stach
Cc: Russell King, David Airlie, Daniel Vetter, Christian Gmeiner,
stable, etnaviv, dri-devel, linux-kernel
Hi Lucas,
>
> prefix for etnaviv kernel patches should be "drm/etnaviv: ..."
Ah yeah .. corrected in v2.
>
> Am Donnerstag, dem 29.02.2024 um 20:55 +0100 schrieb Christian Gmeiner:
> > From: Christian Gmeiner <cgmeiner@igalia.com>
> >
> > The hwdb selection logic as a feature that allows it to mark some fields
> > as 'don't care'. If we match with such a field we memcpy(..)
> > the current etnaviv_chip_identity into ident.
> >
> > This step can overwrite some id values read from the GPU with the
> > 'don't care' value.
> >
> > Fix this issue by restoring the affected values after the memcpy(..).
> >
> > As this is crucial for user space to know when this feature works as
> > expected increment the minor version too.
>
> Uh, right. Lying to userspace about the GPU identity doesn't seem like
> a good idea.
>
> >
> > Fixes: 4078a1186dd3 ("drm/etnaviv: update hwdb selection logic")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Christian Gmeiner <cgmeiner@igalia.com>
> > ---
> > drivers/gpu/drm/etnaviv/etnaviv_drv.c | 2 +-
> > drivers/gpu/drm/etnaviv/etnaviv_hwdb.c | 14 ++++++++++++++
> > 2 files changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > index 6228ce603248..9a2965741dab 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> > @@ -494,7 +494,7 @@ static const struct drm_driver etnaviv_drm_driver = {
> > .desc = "etnaviv DRM",
> > .date = "20151214",
> > .major = 1,
> > - .minor = 3,
> > + .minor = 4,
> > };
> >
> > /*
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> > index 67201242438b..1e38d66702f1 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_hwdb.c
> > @@ -265,6 +265,9 @@ static const struct etnaviv_chip_identity etnaviv_chip_identities[] = {
> > bool etnaviv_fill_identity_from_hwdb(struct etnaviv_gpu *gpu)
> > {
> > struct etnaviv_chip_identity *ident = &gpu->identity;
> > + const u32 product_id = ident->product_id;
> > + const u32 customer_id = ident->customer_id;
> > + const u32 eco_id = ident->eco_id;
> > int i;
> >
> > for (i = 0; i < ARRAY_SIZE(etnaviv_chip_identities); i++) {
> > @@ -278,6 +281,17 @@ bool etnaviv_fill_identity_from_hwdb(struct etnaviv_gpu *gpu)
> > etnaviv_chip_identities[i].eco_id == ~0U)) {
> > memcpy(ident, &etnaviv_chip_identities[i],
> > sizeof(*ident));
> > +
> > + /* Restore some id values if ~0U aka 'don't care' is used. */
> > + if (etnaviv_chip_identities[i].product_id == ~0U)
>
> You can drop all those if clauses. Either the hwdb value is an exact
> match and the value is the same or it's the don't care value and you
> want to restore it, so a simple unconditional assignment would make
> this a bit more compact.
>
Makes sense - thanks for your review!
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-03-01 13:30 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-29 19:55 [PATCH] etnaviv: Restore some id values Christian Gmeiner
2024-03-01 9:27 ` Tomeu Vizoso
2024-03-01 10:08 ` Lucas Stach
2024-03-01 13:30 ` Christian Gmeiner
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.