linux-kernel - Re: [RFC PATCH v5 5/6] drm/tidss: Add IO CTRL and Power support for OLDI TX in am625

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <708ae70e-dc1b-1079-8442-06cbea228e99@ideasonboard.com>
Date:   Wed, 12 Oct 2022 15:29:40 +0300
From:   Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
To:     Aradhya Bhatia <a-bhatia1@...com>, Jyri Sarha <jyri.sarha@....fi>,
        Rob Herring <robh+dt@...nel.org>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>
Cc:     Nishanth Menon <nm@...com>, Vignesh Raghavendra <vigneshr@...com>,
        Rahul T R <r-ravikumar@...com>,
        DRI Development List <dri-devel@...ts.freedesktop.org>,
        Devicetree List <devicetree@...r.kernel.org>,
        Linux Kernel List <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH v5 5/6] drm/tidss: Add IO CTRL and Power support for
 OLDI TX in am625

On 28/09/2022 20:52, Aradhya Bhatia wrote:
> The ctrl mmr module of the AM625 is different from the AM65X SoC. Thus
> the ctrl mmr registers that supported the OLDI TX power have become
> different in AM625 SoC.
> 
> Add IO CTRL support and control the OLDI TX power for AM625.
> 
> Signed-off-by: Aradhya Bhatia <a-bhatia1@...com>
> ---
>   drivers/gpu/drm/tidss/tidss_dispc.c      | 55 ++++++++++++++++++------
>   drivers/gpu/drm/tidss/tidss_dispc_regs.h |  6 +++
>   2 files changed, 49 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c
> index 88008ad39b55..68444e0cd8d7 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc.c
> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c
> @@ -921,21 +921,52 @@ int dispc_vp_bus_check(struct dispc_device *dispc, u32 hw_videoport,
>   
>   static void dispc_oldi_tx_power(struct dispc_device *dispc, bool power)
>   {
> -	u32 val = power ? 0 : OLDI_PWRDN_TX;
> +	u32 val;
>   
>   	if (WARN_ON(!dispc->oldi_io_ctrl))
>   		return;
>   
> -	regmap_update_bits(dispc->oldi_io_ctrl, OLDI_DAT0_IO_CTRL,
> -			   OLDI_PWRDN_TX, val);
> -	regmap_update_bits(dispc->oldi_io_ctrl, OLDI_DAT1_IO_CTRL,
> -			   OLDI_PWRDN_TX, val);
> -	regmap_update_bits(dispc->oldi_io_ctrl, OLDI_DAT2_IO_CTRL,
> -			   OLDI_PWRDN_TX, val);
> -	regmap_update_bits(dispc->oldi_io_ctrl, OLDI_DAT3_IO_CTRL,
> -			   OLDI_PWRDN_TX, val);
> -	regmap_update_bits(dispc->oldi_io_ctrl, OLDI_CLK_IO_CTRL,
> -			   OLDI_PWRDN_TX, val);
> +	if (dispc->feat->subrev == DISPC_AM65X) {
> +		val = power ? 0 : OLDI_PWRDN_TX;
> +
> +		regmap_update_bits(dispc->oldi_io_ctrl, OLDI_DAT0_IO_CTRL,
> +				   OLDI_PWRDN_TX, val);
> +		regmap_update_bits(dispc->oldi_io_ctrl, OLDI_DAT1_IO_CTRL,
> +				   OLDI_PWRDN_TX, val);
> +		regmap_update_bits(dispc->oldi_io_ctrl, OLDI_DAT2_IO_CTRL,
> +				   OLDI_PWRDN_TX, val);
> +		regmap_update_bits(dispc->oldi_io_ctrl, OLDI_DAT3_IO_CTRL,
> +				   OLDI_PWRDN_TX, val);
> +		regmap_update_bits(dispc->oldi_io_ctrl, OLDI_CLK_IO_CTRL,
> +				   OLDI_PWRDN_TX, val);
> +
> +	} else if (dispc->feat->subrev == DISPC_AM625) {
> +		if (power) {
> +			switch (dispc->oldi_mode) {
> +			case OLDI_SINGLE_LINK_SINGLE_MODE:
> +				/* Power down OLDI TX 1 */
> +				val = OLDI1_PWRDN_TX;
> +				break;
> +
> +			case OLDI_SINGLE_LINK_CLONE_MODE:
> +			case OLDI_DUAL_LINK_MODE:
> +				/* No Power down */
> +				val = 0;
> +				break;
> +
> +			default:
> +				/* Power down both the OLDI TXes */
> +				val = OLDI0_PWRDN_TX | OLDI1_PWRDN_TX;
> +				break;
> +			}
> +		} else {
> +			/* Power down both the OLDI TXes */
> +			val = OLDI0_PWRDN_TX | OLDI1_PWRDN_TX;
> +		}

Ugh, I hate power-down bits. So you "enable" it to disable it =). What's 
the default value or the register here? Or will this always be called? 
I.e. if we only use DPI, do we power down the OLDIs somewhere (or does 
it matter)?

> +
> +		regmap_update_bits(dispc->oldi_io_ctrl, OLDI_PD_CTRL,
> +				   OLDI0_PWRDN_TX | OLDI1_PWRDN_TX, val);
> +	}
>   }
>   
>   static void dispc_set_num_datalines(struct dispc_device *dispc,
> @@ -2831,7 +2862,7 @@ int dispc_init(struct tidss_device *tidss)
>   		dispc->vp_data[i].gamma_table = gamma_table;
>   	}
>   
> -	if (feat->subrev == DISPC_AM65X) {
> +	if (feat->subrev == DISPC_AM65X || feat->subrev == DISPC_AM625) {
>   		r = dispc_init_am65x_oldi_io_ctrl(dev, dispc);
>   		if (r)
>   			return r;
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc_regs.h b/drivers/gpu/drm/tidss/tidss_dispc_regs.h
> index 13feedfe5d6d..510bee70b3b8 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc_regs.h
> +++ b/drivers/gpu/drm/tidss/tidss_dispc_regs.h
> @@ -238,6 +238,12 @@ enum dispc_common_regs {
>   #define OLDI_DAT3_IO_CTRL			0x0C
>   #define OLDI_CLK_IO_CTRL			0x10
>   
> +/* Only for AM625 OLDI TX */
> +#define OLDI_PD_CTRL				0x100
> +#define OLDI_LB_CTRL				0x104
> +
>   #define OLDI_PWRDN_TX				BIT(8)
> +#define OLDI0_PWRDN_TX				BIT(0)
> +#define OLDI1_PWRDN_TX				BIT(1)

Maybe these (the new and old ones) should be platform-prefixed. And 
organized so that the register and its bits are together.

  Tomi