Re: [PATCH 05/10] drm/sun4i: csc: Add support for the new MMIO layout

From: "Jernej Škrabec" <jernej.skrabec@gmail.com>
To: Chen-Yu Tsai <wens@csie.org>, Maxime Ripard <mripard@kernel.org>,
	Samuel Holland <samuel@sholland.org>
Cc: Samuel Holland <samuel@sholland.org>,
	Daniel Vetter <daniel@ffwll.ch>, David Airlie <airlied@linux.ie>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	devicetree@vger.kernel.org, dri-devel@lists.freedesktop.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-sunxi@lists.linux.dev
Subject: Re: [PATCH 05/10] drm/sun4i: csc: Add support for the new MMIO layout
Date: Mon, 11 Apr 2022 17:40:38 +0200	[thread overview]
Message-ID: <21425699.EfDdHjke4D@kista> (raw)
In-Reply-To: <20220411043423.37333-6-samuel@sholland.org>

Dne ponedeljek, 11. april 2022 ob 06:34:17 CEST je Samuel Holland napisal(a):
> D1 changes the MMIO address offset for the CSC blocks in the first
> mixer. The ccsc field value is used as an index into the ccsc_base
> array; allocate the next available value to represent the new variant.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
> 
>  drivers/gpu/drm/sun4i/sun8i_csc.c   | 3 ++-
>  drivers/gpu/drm/sun4i/sun8i_csc.h   | 1 +
>  drivers/gpu/drm/sun4i/sun8i_mixer.h | 6 ++++--
>  3 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun8i_csc.c b/drivers/gpu/drm/sun4i/
sun8i_csc.c
> index 9bd62de0c288..1ed10c6447a3 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_csc.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_csc.c
> @@ -8,9 +8,10 @@
>  #include "sun8i_csc.h"
>  #include "sun8i_mixer.h"
>  
> -static const u32 ccsc_base[2][2] = {
> +static const u32 ccsc_base[][2] = {
>  	{CCSC00_OFFSET, CCSC01_OFFSET},
>  	{CCSC10_OFFSET, CCSC11_OFFSET},
> +	{CCSC00_OFFSET, CCSC01_OFFSET_D1},
>  };
>  
>  /*
> diff --git a/drivers/gpu/drm/sun4i/sun8i_csc.h b/drivers/gpu/drm/sun4i/
sun8i_csc.h
> index 022cafa6c06c..bd54166b2bcc 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_csc.h
> +++ b/drivers/gpu/drm/sun4i/sun8i_csc.h
> @@ -13,6 +13,7 @@ struct sun8i_mixer;
>  /* VI channel CSC units offsets */
>  #define CCSC00_OFFSET 0xAA050
>  #define CCSC01_OFFSET 0xFA050
> +#define CCSC01_OFFSET_D1 0xFA000
>  #define CCSC10_OFFSET 0xA0000
>  #define CCSC11_OFFSET 0xF0000
>  
> diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.h b/drivers/gpu/drm/sun4i/
sun8i_mixer.h
> index 5b3fbee18671..22f1b7ef9225 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.h
> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.h
> @@ -151,8 +151,10 @@
>   *	scaler and 2 UI channels with scaler, bitmask would be 0xC.
>   * @ccsc: select set of CCSC base addresses
>   *	Set value to 0 if this is first mixer or second mixer with VEP 
support.
> - *	Set value to 1 if this is second mixer without VEP support. Other 
values
> - *	are invalid.
> + *	Set value to 1 if this is second mixer without VEP support.
> + *	Set value to 2 if this is first mixer or second mixer with VEP 
support,
> + *	and the SoC uses the MMIO layout found in the D1 SoC.
> + *	Other values are invalid.

It would be better to introduce some kind of enum. Number 2 is completely 
arbitrary.

Best regards,
Jernej

>   * @mod_rate: module clock rate that needs to be set in order to have
>   *	a functional block.
>   * @is_de3: true, if this is next gen display engine 3.0, false otherwise.
> -- 
> 2.35.1
> 
> 