Re: [PATCH 17/22] phy: allwinner: phy-sun6i-mipi-dphy: add LVDS support

[Paul Kocialkowski <paulk@sys-base.io>]

[-- Attachment #1: Type: text/plain, Size: 3491 bytes --]

Hi,

Thanks for your work on this!

On Fri 27 Dec 24, 16:38, Parthiban Nallathambi wrote:
> DPHY in A100/A133 supports both LVDS and DSI. Combo phy register
> have BIT(2) for enabling LVDS specifically, but enabling it alone
> isn't functional.
> 
> Both MIPI and LVDS needs to be enabled in the combo phy to get
> the display working under LVDS mode. There is no specific enable
> bit for LVDS apart from the one in combo phy. MIPI got enable
> control in analog 4 register which must be disabled when using
> in LVDS mode.
> 
> Introduce set_mode in phy ops to control only for MIPI DSI.

Similar work was already submitted for D1/T113-S3 LVDS support, which seems to
be the exact same situation as the A133.

See: https://patchwork.freedesktop.org/series/145276/

I just made a review of that series and find it more elegant in various ways
(especially since configuring the registers in set_mode is not the right place).
So you probably want to follow-up on that series instead.

Note that both D1/T113-S3 and A133 support a second LVDS output, LVDS1, which
uses the traditional TCON0 LVDS PHY. It would be great to be able to support
both outputs as well as dual-link modes!

All the best,

Paul

> Signed-off-by: Parthiban Nallathambi <parthiban@linumiz.com>
> ---
>  drivers/phy/allwinner/phy-sun6i-mipi-dphy.c | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/phy/allwinner/phy-sun6i-mipi-dphy.c b/drivers/phy/allwinner/phy-sun6i-mipi-dphy.c
> index 36eab95271b2..d164b2ea5dfd 100644
> --- a/drivers/phy/allwinner/phy-sun6i-mipi-dphy.c
> +++ b/drivers/phy/allwinner/phy-sun6i-mipi-dphy.c
> @@ -314,13 +314,11 @@ static void sun50i_a100_mipi_dphy_tx_power_on(struct sun6i_dphy *dphy)
>  	/* Disable sigma-delta modulation. */
>  	regmap_write(dphy->regs, SUN50I_DPHY_PLL_REG2, 0);
>  
> -	regmap_update_bits(dphy->regs, SUN6I_DPHY_ANA4_REG,
> -			   SUN6I_DPHY_ANA4_REG_EN_MIPI,
> -			   SUN6I_DPHY_ANA4_REG_EN_MIPI);
> -
>  	regmap_update_bits(dphy->regs, SUN50I_COMBO_PHY_REG0,
> +			   SUN50I_COMBO_PHY_REG0_EN_LVDS |
>  			   SUN50I_COMBO_PHY_REG0_EN_MIPI |
>  			   SUN50I_COMBO_PHY_REG0_EN_COMBOLDO,
> +			   SUN50I_COMBO_PHY_REG0_EN_LVDS |
>  			   SUN50I_COMBO_PHY_REG0_EN_MIPI |
>  			   SUN50I_COMBO_PHY_REG0_EN_COMBOLDO);
>  
> @@ -528,6 +526,22 @@ static int sun6i_dphy_exit(struct phy *phy)
>  	return 0;
>  }
>  
> +static int sun6i_set_mode(struct phy *phy, enum phy_mode mode, int submode)
> +{
> +	struct sun6i_dphy *dphy = phy_get_drvdata(phy);
> +
> +	switch (mode) {
> +	case PHY_MODE_MIPI_DPHY:
> +		regmap_update_bits(dphy->regs, SUN6I_DPHY_ANA4_REG,
> +				   SUN6I_DPHY_ANA4_REG_EN_MIPI,
> +				   SUN6I_DPHY_ANA4_REG_EN_MIPI);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
>  
>  static const struct phy_ops sun6i_dphy_ops = {
>  	.configure	= sun6i_dphy_configure,
> @@ -535,6 +549,7 @@ static const struct phy_ops sun6i_dphy_ops = {
>  	.power_off	= sun6i_dphy_power_off,
>  	.init		= sun6i_dphy_init,
>  	.exit		= sun6i_dphy_exit,
> +	.set_mode	= sun6i_set_mode,
>  };
>  
>  static const struct regmap_config sun6i_dphy_regmap_config = {
> 
> -- 
> 2.39.5
> 

-- 
Paul Kocialkowski,

Independent contractor - sys-base - https://www.sys-base.io/
Free software developer - https://www.paulk.fr/

Expert in multimedia, graphics and embedded hardware support with Linux.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]