Re: [PATCH 2/6] drm/etnaviv: determine product, customer and eco id

[Lucas Stach]

On Do, 2020-01-02 at 11:02 +0100, Christian Gmeiner wrote:
> They will be used for extended HWDB support. The eco id logic was taken
> from galcore kernel driver sources.
> 
> Signed-off-by: Christian Gmeiner <christian.gmeiner@xxxxxxxxx>
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 17 +++++++++++++++++
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.h |  6 +++---
>  2 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index d47d1a8e0219..253301be9e95 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -321,6 +321,18 @@ static void etnaviv_hw_specs(struct etnaviv_gpu *gpu)
>  		gpu->identity.varyings_count -= 1;
>  }
>  
> +static void etnaviv_hw_eco_id(struct etnaviv_gpu *gpu)
> +{
> +	const u32 chipDate = gpu_read(gpu, VIVS_HI_CHIP_DATE);
> +	gpu->identity.eco_id = gpu_read(gpu, VIVS_HI_CHIP_ECO_ID);
> +
> +	if (etnaviv_is_model_rev(gpu, GC1000, 0x5037) && (chipDate == 0x20120617))
> +		gpu->identity.eco_id = 1;
> +
> +	if (etnaviv_is_model_rev(gpu, GC320, 0x5303) && (chipDate == 0x20140511))
> +		gpu->identity.eco_id = 1;

I'm not sure if those two checks warrant a separate function. Maybe
just place them besides the other ID fixups?

> +}
> +
>  static void etnaviv_hw_identify(struct etnaviv_gpu *gpu)
>  {
>  	u32 chipIdentity;
> @@ -362,6 +374,8 @@ static void etnaviv_hw_identify(struct etnaviv_gpu *gpu)
>  			}
>  		}
>  
> +		gpu->identity.product_id = gpu_read(gpu, VIVS_HI_CHIP_PRODUCT_ID);
> +
>  		/*
>  		 * NXP likes to call the GPU on the i.MX6QP GC2000+, but in
>  		 * reality it's just a re-branded GC3000. We can identify this
> @@ -375,6 +389,9 @@ static void etnaviv_hw_identify(struct etnaviv_gpu *gpu)
>  		}
>  	}
>  
> +	etnaviv_hw_eco_id(gpu);
> +	gpu->identity.customer_id = gpu_read(gpu, VIVS_HI_CHIP_CUSTOMER_ID);

I don't like this scattering of identity register reads. Please move
all of those reads to the else clause where we currently read
model/rev. I doubt that the customer ID register is available on the
really early cores, that only have the VIVS_HI_CHIP_IDENTITY register.

Regards,
Lucas

>  	dev_info(gpu->dev, "model: GC%x, revision: %x\n",
>  		 gpu->identity.model, gpu->identity.revision);
>  
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> index 8f9bd4edc96a..68bd966e3916 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> @@ -15,11 +15,11 @@ struct etnaviv_gem_submit;
>  struct etnaviv_vram_mapping;
>  
>  struct etnaviv_chip_identity {
> -	/* Chip model. */
>  	u32 model;
> -
> -	/* Revision value.*/
>  	u32 revision;
> +	u32 product_id;
> +	u32 customer_id;
> +	u32 eco_id;
>  
>  	/* Supported feature fields. */
>  	u32 features;