From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@kernel.org>
X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on
	aws-us-west-2-korg-lkml-1.web.codeaurora.org
X-Spam-Level: 
X-Spam-Status: No, score=-15.3 required=3.0 tests=BAYES_00,
	HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH,
	MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY,
	USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0
Received: from mail.kernel.org (mail.kernel.org [198.145.29.99])
	by smtp.lore.kernel.org (Postfix) with ESMTP id 877DCC433E0
	for <linux-kernel@archiver.kernel.org>; Mon, 15 Feb 2021 14:36:09 +0000 (UTC)
Received: from vger.kernel.org (vger.kernel.org [23.128.96.18])
	by mail.kernel.org (Postfix) with ESMTP id 575EF64E40
	for <linux-kernel@archiver.kernel.org>; Mon, 15 Feb 2021 14:36:09 +0000 (UTC)
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S230100AbhBOOfp (ORCPT <rfc822;linux-kernel@archiver.kernel.org>);
        Mon, 15 Feb 2021 09:35:45 -0500
Received: from bhuna.collabora.co.uk ([46.235.227.227]:59418 "EHLO
        bhuna.collabora.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S230196AbhBOOeg (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Mon, 15 Feb 2021 09:34:36 -0500
Received: from [127.0.0.1] (localhost [127.0.0.1])
        (Authenticated sender: koike)
        with ESMTPSA id C14871F44DE7
Subject: Re: [PATCH 3/6] drm/rockchip: dsi: add ability to work as a phy
 instead of full dsi
To:     Heiko Stuebner <heiko@sntech.de>, dri-devel@lists.freedesktop.org
Cc:     devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org,
        hjc@rock-chips.com, robh+dt@kernel.org,
        sebastian.fricke@posteo.net, linux-media@vger.kernel.org,
        dafna.hirschfeld@collabora.com, ezequiel@collabora.com,
        cmuellner@linux.com,
        Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
References: <20210210111020.2476369-1-heiko@sntech.de>
 <20210210111020.2476369-4-heiko@sntech.de>
From:   Helen Koike <helen.koike@collabora.com>
Message-ID: <d7b8137c-66ce-935a-c8d7-e507146143d7@collabora.com>
Date:   Mon, 15 Feb 2021 11:33:19 -0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.7.1
MIME-Version: 1.0
In-Reply-To: <20210210111020.2476369-4-heiko@sntech.de>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org



On 2/10/21 8:10 AM, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> 
> SoCs like the rk3288 and rk3399 have 3 mipi dphys on them. One is TX-
> only, one is RX-only and one can be configured to do either TX or RX.
> 
> The RX phy is statically connected to the first Image Signal Processor,
> the TX phy is statically connected to the first DSI controller and
> the TXRX phy is connected to both the second DSI controller as well
> as the second ISP.
> 
> The RX dphy is controlled externally through registers in the "General
> Register Files", while the other two are controlled through the
> "Configuration and Test Interface" inside their DSI controller's
> io-memory area.
> 
> The Rockchip dw-dsi controller already controls these dphys for the
> TX case in the driver, but when we want to also allow configuration
> for RX to the ISP from the media subsystem we need to expose phy-
> functionality instead.
> 
> So add a bit of infrastructure to allow the dsi driver to work as a
> phy and make sure it can be only one or the other at a time.
> 
> Similarly as the dsi-controller will be part of the drm-graph when
> active, add an empty component to the drm-graph when in phy-mode
> to make the rest of the drm-graph not wait for it.
> 
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> Tested-by: Sebastian Fricke <sebastian.fricke@posteo.net>
> ---
>   drivers/gpu/drm/rockchip/Kconfig              |   2 +
>   .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c   | 341 ++++++++++++++++++
>   2 files changed, 343 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
> index cb25c0e8fc9b..3094d4533ad6 100644
> --- a/drivers/gpu/drm/rockchip/Kconfig
> +++ b/drivers/gpu/drm/rockchip/Kconfig
> @@ -9,6 +9,8 @@ config DRM_ROCKCHIP
>   	select DRM_ANALOGIX_DP if ROCKCHIP_ANALOGIX_DP
>   	select DRM_DW_HDMI if ROCKCHIP_DW_HDMI
>   	select DRM_DW_MIPI_DSI if ROCKCHIP_DW_MIPI_DSI
> +	select GENERIC_PHY if ROCKCHIP_DW_MIPI_DSI
> +	select GENERIC_PHY_MIPI_DPHY if ROCKCHIP_DW_MIPI_DSI

maybe alphabetical order?

>   	select DRM_RGB if ROCKCHIP_RGB
>   	select SND_SOC_HDMI_CODEC if ROCKCHIP_CDN_DP && SND_SOC
>   	help
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> index 18e112e30f6e..e322749a5279 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> @@ -14,6 +14,7 @@
>   #include <linux/of_device.h>
>   #include <linux/phy/phy.h>
>   #include <linux/pm_runtime.h>
> +#include <linux/phy/phy.h>
>   #include <linux/regmap.h>
>   
>   #include <video/mipi_display.h>
> @@ -125,7 +126,9 @@
>   #define BANDGAP_AND_BIAS_CONTROL			0x20
>   #define TERMINATION_RESISTER_CONTROL			0x21
>   #define AFE_BIAS_BANDGAP_ANALOG_PROGRAMMABILITY		0x22
> +#define HS_RX_CONTROL_OF_LANE_CLK			0x34
>   #define HS_RX_CONTROL_OF_LANE_0				0x44
> +#define HS_RX_CONTROL_OF_LANE_1				0x54
>   #define HS_TX_CLOCK_LANE_REQUEST_STATE_TIME_CONTROL	0x60
>   #define HS_TX_CLOCK_LANE_PREPARE_STATE_TIME_CONTROL	0x61
>   #define HS_TX_CLOCK_LANE_HS_ZERO_STATE_TIME_CONTROL	0x62
> @@ -137,6 +140,9 @@
>   #define HS_TX_DATA_LANE_HS_ZERO_STATE_TIME_CONTROL	0x72
>   #define HS_TX_DATA_LANE_TRAIL_STATE_TIME_CONTROL	0x73
>   #define HS_TX_DATA_LANE_EXIT_STATE_TIME_CONTROL		0x74
> +#define HS_RX_DATA_LANE_THS_SETTLE_CONTROL		0x75
> +#define HS_RX_CONTROL_OF_LANE_2				0x84
> +#define HS_RX_CONTROL_OF_LANE_3				0x94
>   
>   #define DW_MIPI_NEEDS_PHY_CFG_CLK	BIT(0)
>   #define DW_MIPI_NEEDS_GRF_CLK		BIT(1)
> @@ -171,11 +177,19 @@
>   #define RK3399_TXRX_MASTERSLAVEZ	BIT(7)
>   #define RK3399_TXRX_ENABLECLK		BIT(6)
>   #define RK3399_TXRX_BASEDIR		BIT(5)
> +#define RK3399_TXRX_SRC_SEL_ISP0	BIT(4)
> +#define RK3399_TXRX_TURNREQUEST		GENMASK(3, 0)
>   
>   #define HIWORD_UPDATE(val, mask)	(val | (mask) << 16)
>   
>   #define to_dsi(nm)	container_of(nm, struct dw_mipi_dsi_rockchip, nm)
>   
> +enum {
> +	DW_DSI_USAGE_IDLE,
> +	DW_DSI_USAGE_DSI,
> +	DW_DSI_USAGE_PHY,
> +};
> +
>   enum {
>   	BANDGAP_97_07,
>   	BANDGAP_98_05,
> @@ -213,6 +227,10 @@ struct rockchip_dw_dsi_chip_data {
>   	u32 lanecfg2_grf_reg;
>   	u32 lanecfg2;
>   
> +	int (*dphy_rx_init)(struct phy *phy);
> +	int (*dphy_rx_power_on)(struct phy *phy);
> +	int (*dphy_rx_power_off)(struct phy *phy);
> +
>   	unsigned int flags;
>   	unsigned int max_data_lanes;
>   };
> @@ -236,6 +254,12 @@ struct dw_mipi_dsi_rockchip {
>   	struct phy *phy;
>   	union phy_configure_opts phy_opts;
>   
> +	/* being a phy for other mipi hosts */
> +	unsigned int usage_mode;
> +	struct mutex usage_mutex;
> +	struct phy *dphy;
> +	struct phy_configure_opts_mipi_dphy dphy_config;
> +
>   	unsigned int lane_mbps; /* per lane */
>   	u16 input_div;
>   	u16 feedback_div;
> @@ -965,6 +989,17 @@ static int dw_mipi_dsi_rockchip_host_attach(void *priv_data,
>   	struct device *second;
>   	int ret;
>   
> +	mutex_lock(&dsi->usage_mutex);
> +
> +	if (dsi->usage_mode != DW_DSI_USAGE_IDLE) {
> +		DRM_DEV_ERROR(dsi->dev, "dsi controller already in use\n");
> +		mutex_unlock(&dsi->usage_mutex);
> +		return -EBUSY;
> +	}
> +
> +	dsi->usage_mode = DW_DSI_USAGE_DSI;
> +	mutex_unlock(&dsi->usage_mutex);
> +
>   	ret = component_add(dsi->dev, &dw_mipi_dsi_rockchip_ops);
>   	if (ret) {
>   		DRM_DEV_ERROR(dsi->dev, "Failed to register component: %d\n",
> @@ -1000,6 +1035,10 @@ static int dw_mipi_dsi_rockchip_host_detach(void *priv_data,
>   
>   	component_del(dsi->dev, &dw_mipi_dsi_rockchip_ops);
>   
> +	mutex_lock(&dsi->usage_mutex);
> +	dsi->usage_mode = DW_DSI_USAGE_IDLE;
> +	mutex_unlock(&dsi->usage_mutex);
> +
>   	return 0;
>   }
>   
> @@ -1008,11 +1047,227 @@ static const struct dw_mipi_dsi_host_ops dw_mipi_dsi_rockchip_host_ops = {
>   	.detach = dw_mipi_dsi_rockchip_host_detach,
>   };
>   
> +static int dw_mipi_dsi_rockchip_dphy_bind(struct device *dev,
> +					  struct device *master,
> +					  void *data)
> +{
> +	/*
> +	 * Nothing to do when used as a dphy.
> +	 * Just make the rest of Rockchip-DRM happy
> +	 * by being here.
> +	 */
> +
> +	return 0;
> +}
> +
> +static void dw_mipi_dsi_rockchip_dphy_unbind(struct device *dev,
> +					     struct device *master,
> +					     void *data)
> +{
> +	/* Nothing to do when used as a dphy. */
> +}
> +
> +static const struct component_ops dw_mipi_dsi_rockchip_dphy_ops = {
> +	.bind	= dw_mipi_dsi_rockchip_dphy_bind,
> +	.unbind	= dw_mipi_dsi_rockchip_dphy_unbind,
> +};
> +
> +static int dw_mipi_dsi_dphy_init(struct phy *phy)
> +{
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +	int ret;
> +
> +	mutex_lock(&dsi->usage_mutex);
> +
> +	if (dsi->usage_mode != DW_DSI_USAGE_IDLE) {
> +		DRM_DEV_ERROR(dsi->dev, "dsi controller already in use\n");
> +		mutex_unlock(&dsi->usage_mutex);
> +		return -EBUSY;
> +	}
> +
> +	dsi->usage_mode = DW_DSI_USAGE_PHY;
> +	mutex_unlock(&dsi->usage_mutex);
> +
> +	ret = component_add(dsi->dev, &dw_mipi_dsi_rockchip_dphy_ops);
> +	if (ret < 0)
> +		goto err_graph;
> +
> +	if (dsi->cdata->dphy_rx_init) {
> +		ret = clk_prepare_enable(dsi->pclk);
> +		if (ret < 0)
> +			goto err_init;
> +
> +		ret = clk_prepare_enable(dsi->grf_clk);
> +		if (ret) {
> +			clk_disable_unprepare(dsi->pclk);
> +			goto err_init;
> +		}
> +
> +		ret = dsi->cdata->dphy_rx_init(phy);
> +		clk_disable_unprepare(dsi->grf_clk);
> +		clk_disable_unprepare(dsi->pclk);
> +		if (ret < 0)
> +			goto err_init;
> +	}
> +
> +	return 0;
> +
> +err_init:
> +	component_del(dsi->dev, &dw_mipi_dsi_rockchip_dphy_ops);
> +err_graph:
> +	mutex_lock(&dsi->usage_mutex);
> +	dsi->usage_mode = DW_DSI_USAGE_IDLE;
> +	mutex_unlock(&dsi->usage_mutex);
> +
> +	return ret;
> +}
> +
> +static int dw_mipi_dsi_dphy_exit(struct phy *phy)
> +{
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +
> +	component_del(dsi->dev, &dw_mipi_dsi_rockchip_dphy_ops);
> +
> +	mutex_lock(&dsi->usage_mutex);
> +	dsi->usage_mode = DW_DSI_USAGE_IDLE;
> +	mutex_unlock(&dsi->usage_mutex);
> +
> +	return 0;
> +}
> +
> +static int dw_mipi_dsi_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
> +{
> +	struct phy_configure_opts_mipi_dphy *config = &opts->mipi_dphy;
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +	int ret;
> +
> +	ret = phy_mipi_dphy_config_validate(&opts->mipi_dphy);
> +	if (ret)
> +		return ret;
> +
> +	dsi->dphy_config = *config;
> +	dsi->lane_mbps = div_u64(config->hs_clk_rate, 1000 * 1000 * 1);
> +
> +	return 0;
> +}
> +
> +static int dw_mipi_dsi_dphy_power_on(struct phy *phy)
> +{
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +	int i, ret;

It seems "i" could be removed, use ret instead.

In general, the patch doesn't look wrong to me.

For the whole serie:
Acked-by: Helen Koike <helen.koike@collabora.com>

Thanks
Helen

> +
> +	DRM_DEV_DEBUG(dsi->dev, "lanes %d - data_rate_mbps %u\n",
> +		      dsi->dphy_config.lanes, dsi->lane_mbps);
> +
> +	i = max_mbps_to_parameter(dsi->lane_mbps);
> +	if (i < 0) {
> +		DRM_DEV_ERROR(dsi->dev, "failed to get parameter for %dmbps clock\n",
> +			      dsi->lane_mbps);
> +		return i;
> +	}
> +
> +	ret = pm_runtime_get_sync(dsi->dev);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(dsi->dev, "failed to enable device: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(dsi->pclk);
> +	if (ret) {
> +		DRM_DEV_ERROR(dsi->dev, "Failed to enable pclk: %d\n", ret);
> +		goto err_pclk;
> +	}
> +
> +	ret = clk_prepare_enable(dsi->grf_clk);
> +	if (ret) {
> +		DRM_DEV_ERROR(dsi->dev, "Failed to enable grf_clk: %d\n", ret);
> +		goto err_grf_clk;
> +	}
> +
> +	ret = clk_prepare_enable(dsi->phy_cfg_clk);
> +	if (ret) {
> +		DRM_DEV_ERROR(dsi->dev, "Failed to enable phy_cfg_clk: %d\n", ret);
> +		goto err_phy_cfg_clk;
> +	}
> +
> +	/* do soc-variant specific init */
> +	if (dsi->cdata->dphy_rx_power_on) {
> +		ret = dsi->cdata->dphy_rx_power_on(phy);
> +		if (ret < 0) {
> +			DRM_DEV_ERROR(dsi->dev, "hardware-specific phy bringup failed: %d\n", ret);
> +			goto err_pwr_on;
> +		}
> +	}
> +
> +	/*
> +	 * Configure hsfreqrange according to frequency values
> +	 * Set clock lane and hsfreqrange by lane0(test code 0x44)
> +	 */
> +	dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_CLK, 0);
> +	dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_0,
> +			      HSFREQRANGE_SEL(dppa_map[i].hsfreqrange));
> +	dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_1, 0);
> +	dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_2, 0);
> +	dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_3, 0);
> +
> +	/* Normal operation */
> +	dw_mipi_dsi_phy_write(dsi, 0x0, 0);
> +
> +	clk_disable_unprepare(dsi->phy_cfg_clk);
> +	clk_disable_unprepare(dsi->grf_clk);
> +
> +	return ret;
> +
> +err_pwr_on:
> +	clk_disable_unprepare(dsi->phy_cfg_clk);
> +err_phy_cfg_clk:
> +	clk_disable_unprepare(dsi->grf_clk);
> +err_grf_clk:
> +	clk_disable_unprepare(dsi->pclk);
> +err_pclk:
> +	pm_runtime_put(dsi->dev);
> +	return ret;
> +}
> +
> +static int dw_mipi_dsi_dphy_power_off(struct phy *phy)
> +{
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +	int ret;
> +
> +	ret = clk_prepare_enable(dsi->grf_clk);
> +	if (ret) {
> +		DRM_DEV_ERROR(dsi->dev, "Failed to enable grf_clk: %d\n", ret);
> +		return ret;
> +	}
> +
> +	if (dsi->cdata->dphy_rx_power_off) {
> +		ret = dsi->cdata->dphy_rx_power_off(phy);
> +		if (ret < 0)
> +			DRM_DEV_ERROR(dsi->dev, "hardware-specific phy shutdown failed: %d\n", ret);
> +	}
> +
> +	clk_disable_unprepare(dsi->grf_clk);
> +	clk_disable_unprepare(dsi->pclk);
> +
> +	pm_runtime_put(dsi->dev);
> +
> +	return ret;
> +}
> +
> +static const struct phy_ops dw_mipi_dsi_dphy_ops = {
> +	.configure	= dw_mipi_dsi_dphy_configure,
> +	.power_on	= dw_mipi_dsi_dphy_power_on,
> +	.power_off	= dw_mipi_dsi_dphy_power_off,
> +	.init		= dw_mipi_dsi_dphy_init,
> +	.exit		= dw_mipi_dsi_dphy_exit,
> +};
> +
>   static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev)
>   {
>   	struct device *dev = &pdev->dev;
>   	struct device_node *np = dev->of_node;
>   	struct dw_mipi_dsi_rockchip *dsi;
> +	struct phy_provider *phy_provider;
>   	struct resource *res;
>   	const struct rockchip_dw_dsi_chip_data *cdata =
>   				of_device_get_match_data(dev);
> @@ -1109,6 +1364,19 @@ static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev)
>   	dsi->pdata.priv_data = dsi;
>   	platform_set_drvdata(pdev, dsi);
>   
> +	mutex_init(&dsi->usage_mutex);
> +
> +	dsi->dphy = devm_phy_create(dev, NULL, &dw_mipi_dsi_dphy_ops);
> +	if (IS_ERR(dsi->dphy)) {
> +		DRM_DEV_ERROR(&pdev->dev, "failed to create PHY\n");
> +		return PTR_ERR(dsi->dphy);
> +	}
> +
> +	phy_set_drvdata(dsi->dphy, dsi);
> +	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +	if (IS_ERR(phy_provider))
> +		return PTR_ERR(phy_provider);
> +
>   	dsi->dmd = dw_mipi_dsi_probe(pdev, &dsi->pdata);
>   	if (IS_ERR(dsi->dmd)) {
>   		ret = PTR_ERR(dsi->dmd);
> @@ -1175,6 +1443,75 @@ static const struct rockchip_dw_dsi_chip_data rk3288_chip_data[] = {
>   	{ /* sentinel */ }
>   };
>   
> +static int rk3399_dphy_tx1rx1_init(struct phy *phy)
> +{
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +
> +	/*
> +	 * Set TX1RX1 source to isp1.
> +	 * Assume ISP0 is supplied by the RX0 dphy.
> +	 */
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON24,
> +		     HIWORD_UPDATE(0, RK3399_TXRX_SRC_SEL_ISP0));
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON24,
> +		     HIWORD_UPDATE(0, RK3399_TXRX_MASTERSLAVEZ));
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON24,
> +		     HIWORD_UPDATE(0, RK3399_TXRX_BASEDIR));
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON23,
> +		     HIWORD_UPDATE(0, RK3399_DSI1_ENABLE));
> +
> +	return 0;
> +}
> +
> +static int rk3399_dphy_tx1rx1_power_on(struct phy *phy)
> +{
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +
> +	/* tester reset pulse */
> +	dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLK | PHY_TESTCLR);
> +	usleep_range(100, 150);
> +
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON24,
> +		     HIWORD_UPDATE(0, RK3399_TXRX_MASTERSLAVEZ));
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON24,
> +		     HIWORD_UPDATE(RK3399_TXRX_BASEDIR, RK3399_TXRX_BASEDIR));
> +
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON23,
> +		     HIWORD_UPDATE(0, RK3399_DSI1_FORCERXMODE));
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON23,
> +		     HIWORD_UPDATE(0, RK3399_DSI1_FORCETXSTOPMODE));
> +
> +	/* Disable lane turn around, which is ignored in receive mode */
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON24,
> +		     HIWORD_UPDATE(0, RK3399_TXRX_TURNREQUEST));
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON23,
> +		     HIWORD_UPDATE(RK3399_DSI1_TURNDISABLE,
> +				   RK3399_DSI1_TURNDISABLE));
> +	usleep_range(100, 150);
> +
> +	dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLK | PHY_UNTESTCLR);
> +	usleep_range(100, 150);
> +
> +	/* Enable dphy lanes */
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON23,
> +		     HIWORD_UPDATE(GENMASK(dsi->dphy_config.lanes - 1, 0),
> +				   RK3399_DSI1_ENABLE));
> +
> +	usleep_range(100, 150);
> +
> +	return 0;
> +}
> +
> +static int rk3399_dphy_tx1rx1_power_off(struct phy *phy)
> +{
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON23,
> +		     HIWORD_UPDATE(0, RK3399_DSI1_ENABLE));
> +
> +	return 0;
> +}
> +
>   static const struct rockchip_dw_dsi_chip_data rk3399_chip_data[] = {
>   	{
>   		.reg = 0xff960000,
> @@ -1217,6 +1554,10 @@ static const struct rockchip_dw_dsi_chip_data rk3399_chip_data[] = {
>   
>   		.flags = DW_MIPI_NEEDS_PHY_CFG_CLK | DW_MIPI_NEEDS_GRF_CLK,
>   		.max_data_lanes = 4,
> +
> +		.dphy_rx_init = rk3399_dphy_tx1rx1_init,
> +		.dphy_rx_power_on = rk3399_dphy_tx1rx1_power_on,
> +		.dphy_rx_power_off = rk3399_dphy_tx1rx1_power_off,
>   	},
>   	{ /* sentinel */ }
>   };
> 

From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <SRS0=FO72=HR=lists.infradead.org=linux-rockchip-bounces+linux-rockchip=archiver.kernel.org@kernel.org>
X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on
	aws-us-west-2-korg-lkml-1.web.codeaurora.org
X-Spam-Level: 
X-Spam-Status: No, score=-15.3 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH,
	DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,
	INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,
	UNPARSEABLE_RELAY,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no
	version=3.4.0
Received: from mail.kernel.org (mail.kernel.org [198.145.29.99])
	by smtp.lore.kernel.org (Postfix) with ESMTP id 62A10C433DB
	for <linux-rockchip@archiver.kernel.org>; Mon, 15 Feb 2021 14:33:43 +0000 (UTC)
Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(No client certificate requested)
	by mail.kernel.org (Postfix) with ESMTPS id 14705614A5
	for <linux-rockchip@archiver.kernel.org>; Mon, 15 Feb 2021 14:33:43 +0000 (UTC)
DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 14705614A5
Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=collabora.com
Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-rockchip-bounces+linux-rockchip=archiver.kernel.org@lists.infradead.org
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;
	d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Type:
	Content-Transfer-Encoding:Cc:List-Subscribe:List-Help:List-Post:List-Archive:
	List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From:
	References:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date:
	Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner;
	 bh=icga+9y1Qxbd7N+iBt6+WoutEcNWVRvolmMJPbsHcqU=; b=nmcwQQ0BPOpPccbON/wpfWU2H
	RS9Um4l0BrKWc4MwucbHGVZhJsJnbtTwwPyIFXBEuCb3gAYXS52vxWbxybh89CId/hBf0aHE7PvC0
	L+Rd4QsmUY1al0KxXm3cMNfWD+rhyHbkTh0Tgyye7hFZOU7FAYCvRk4pCzhCEmjgBHO6ZC4+coRai
	ruCTgqxXfViRRyBXGME0EQlNim4loRA1VMK7ZtEAMCS9WDP979n80UoEpUmuErbAiLN/i4H0mo/K/
	/0ZbluQuBjS9C4a72GCwD9O8zOcPvtdZpS4Ves5IZLrK/FCErE1H13DzTkdrAu6FqFRA5mEmq96Jz
	JGp3On8Ig==;
Received: from localhost ([::1] helo=merlin.infradead.org)
	by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux))
	id 1lBewT-0005Tm-OM; Mon, 15 Feb 2021 14:33:37 +0000
Received: from bhuna.collabora.co.uk ([46.235.227.227])
 by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux))
 id 1lBewO-0005T5-OE; Mon, 15 Feb 2021 14:33:34 +0000
Received: from [127.0.0.1] (localhost [127.0.0.1])
 (Authenticated sender: koike) with ESMTPSA id C14871F44DE7
Subject: Re: [PATCH 3/6] drm/rockchip: dsi: add ability to work as a phy
 instead of full dsi
To: Heiko Stuebner <heiko@sntech.de>, dri-devel@lists.freedesktop.org
References: <20210210111020.2476369-1-heiko@sntech.de>
 <20210210111020.2476369-4-heiko@sntech.de>
From: Helen Koike <helen.koike@collabora.com>
Message-ID: <d7b8137c-66ce-935a-c8d7-e507146143d7@collabora.com>
Date: Mon, 15 Feb 2021 11:33:19 -0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.7.1
MIME-Version: 1.0
In-Reply-To: <20210210111020.2476369-4-heiko@sntech.de>
Content-Language: en-US
X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 
X-CRM114-CacheID: sfid-20210215_093333_015138_7062A578 
X-CRM114-Status: GOOD (  38.18  )
X-BeenThere: linux-rockchip@lists.infradead.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Upstream kernel work for Rockchip platforms
 <linux-rockchip.lists.infradead.org>
List-Unsubscribe: <http://lists.infradead.org/mailman/options/linux-rockchip>, 
 <mailto:linux-rockchip-request@lists.infradead.org?subject=unsubscribe>
List-Archive: <http://lists.infradead.org/pipermail/linux-rockchip/>
List-Post: <mailto:linux-rockchip@lists.infradead.org>
List-Help: <mailto:linux-rockchip-request@lists.infradead.org?subject=help>
List-Subscribe: <http://lists.infradead.org/mailman/listinfo/linux-rockchip>, 
 <mailto:linux-rockchip-request@lists.infradead.org?subject=subscribe>
Cc: devicetree@vger.kernel.org, dafna.hirschfeld@collabora.com,
 cmuellner@linux.com, hjc@rock-chips.com, linux-kernel@vger.kernel.org,
 linux-rockchip@lists.infradead.org, robh+dt@kernel.org,
 sebastian.fricke@posteo.net,
 Heiko Stuebner <heiko.stuebner@theobroma-systems.com>, ezequiel@collabora.com,
 linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Sender: "Linux-rockchip" <linux-rockchip-bounces@lists.infradead.org>
Errors-To: linux-rockchip-bounces+linux-rockchip=archiver.kernel.org@lists.infradead.org



On 2/10/21 8:10 AM, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> 
> SoCs like the rk3288 and rk3399 have 3 mipi dphys on them. One is TX-
> only, one is RX-only and one can be configured to do either TX or RX.
> 
> The RX phy is statically connected to the first Image Signal Processor,
> the TX phy is statically connected to the first DSI controller and
> the TXRX phy is connected to both the second DSI controller as well
> as the second ISP.
> 
> The RX dphy is controlled externally through registers in the "General
> Register Files", while the other two are controlled through the
> "Configuration and Test Interface" inside their DSI controller's
> io-memory area.
> 
> The Rockchip dw-dsi controller already controls these dphys for the
> TX case in the driver, but when we want to also allow configuration
> for RX to the ISP from the media subsystem we need to expose phy-
> functionality instead.
> 
> So add a bit of infrastructure to allow the dsi driver to work as a
> phy and make sure it can be only one or the other at a time.
> 
> Similarly as the dsi-controller will be part of the drm-graph when
> active, add an empty component to the drm-graph when in phy-mode
> to make the rest of the drm-graph not wait for it.
> 
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> Tested-by: Sebastian Fricke <sebastian.fricke@posteo.net>
> ---
>   drivers/gpu/drm/rockchip/Kconfig              |   2 +
>   .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c   | 341 ++++++++++++++++++
>   2 files changed, 343 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
> index cb25c0e8fc9b..3094d4533ad6 100644
> --- a/drivers/gpu/drm/rockchip/Kconfig
> +++ b/drivers/gpu/drm/rockchip/Kconfig
> @@ -9,6 +9,8 @@ config DRM_ROCKCHIP
>   	select DRM_ANALOGIX_DP if ROCKCHIP_ANALOGIX_DP
>   	select DRM_DW_HDMI if ROCKCHIP_DW_HDMI
>   	select DRM_DW_MIPI_DSI if ROCKCHIP_DW_MIPI_DSI
> +	select GENERIC_PHY if ROCKCHIP_DW_MIPI_DSI
> +	select GENERIC_PHY_MIPI_DPHY if ROCKCHIP_DW_MIPI_DSI

maybe alphabetical order?

>   	select DRM_RGB if ROCKCHIP_RGB
>   	select SND_SOC_HDMI_CODEC if ROCKCHIP_CDN_DP && SND_SOC
>   	help
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> index 18e112e30f6e..e322749a5279 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> @@ -14,6 +14,7 @@
>   #include <linux/of_device.h>
>   #include <linux/phy/phy.h>
>   #include <linux/pm_runtime.h>
> +#include <linux/phy/phy.h>
>   #include <linux/regmap.h>
>   
>   #include <video/mipi_display.h>
> @@ -125,7 +126,9 @@
>   #define BANDGAP_AND_BIAS_CONTROL			0x20
>   #define TERMINATION_RESISTER_CONTROL			0x21
>   #define AFE_BIAS_BANDGAP_ANALOG_PROGRAMMABILITY		0x22
> +#define HS_RX_CONTROL_OF_LANE_CLK			0x34
>   #define HS_RX_CONTROL_OF_LANE_0				0x44
> +#define HS_RX_CONTROL_OF_LANE_1				0x54
>   #define HS_TX_CLOCK_LANE_REQUEST_STATE_TIME_CONTROL	0x60
>   #define HS_TX_CLOCK_LANE_PREPARE_STATE_TIME_CONTROL	0x61
>   #define HS_TX_CLOCK_LANE_HS_ZERO_STATE_TIME_CONTROL	0x62
> @@ -137,6 +140,9 @@
>   #define HS_TX_DATA_LANE_HS_ZERO_STATE_TIME_CONTROL	0x72
>   #define HS_TX_DATA_LANE_TRAIL_STATE_TIME_CONTROL	0x73
>   #define HS_TX_DATA_LANE_EXIT_STATE_TIME_CONTROL		0x74
> +#define HS_RX_DATA_LANE_THS_SETTLE_CONTROL		0x75
> +#define HS_RX_CONTROL_OF_LANE_2				0x84
> +#define HS_RX_CONTROL_OF_LANE_3				0x94
>   
>   #define DW_MIPI_NEEDS_PHY_CFG_CLK	BIT(0)
>   #define DW_MIPI_NEEDS_GRF_CLK		BIT(1)
> @@ -171,11 +177,19 @@
>   #define RK3399_TXRX_MASTERSLAVEZ	BIT(7)
>   #define RK3399_TXRX_ENABLECLK		BIT(6)
>   #define RK3399_TXRX_BASEDIR		BIT(5)
> +#define RK3399_TXRX_SRC_SEL_ISP0	BIT(4)
> +#define RK3399_TXRX_TURNREQUEST		GENMASK(3, 0)
>   
>   #define HIWORD_UPDATE(val, mask)	(val | (mask) << 16)
>   
>   #define to_dsi(nm)	container_of(nm, struct dw_mipi_dsi_rockchip, nm)
>   
> +enum {
> +	DW_DSI_USAGE_IDLE,
> +	DW_DSI_USAGE_DSI,
> +	DW_DSI_USAGE_PHY,
> +};
> +
>   enum {
>   	BANDGAP_97_07,
>   	BANDGAP_98_05,
> @@ -213,6 +227,10 @@ struct rockchip_dw_dsi_chip_data {
>   	u32 lanecfg2_grf_reg;
>   	u32 lanecfg2;
>   
> +	int (*dphy_rx_init)(struct phy *phy);
> +	int (*dphy_rx_power_on)(struct phy *phy);
> +	int (*dphy_rx_power_off)(struct phy *phy);
> +
>   	unsigned int flags;
>   	unsigned int max_data_lanes;
>   };
> @@ -236,6 +254,12 @@ struct dw_mipi_dsi_rockchip {
>   	struct phy *phy;
>   	union phy_configure_opts phy_opts;
>   
> +	/* being a phy for other mipi hosts */
> +	unsigned int usage_mode;
> +	struct mutex usage_mutex;
> +	struct phy *dphy;
> +	struct phy_configure_opts_mipi_dphy dphy_config;
> +
>   	unsigned int lane_mbps; /* per lane */
>   	u16 input_div;
>   	u16 feedback_div;
> @@ -965,6 +989,17 @@ static int dw_mipi_dsi_rockchip_host_attach(void *priv_data,
>   	struct device *second;
>   	int ret;
>   
> +	mutex_lock(&dsi->usage_mutex);
> +
> +	if (dsi->usage_mode != DW_DSI_USAGE_IDLE) {
> +		DRM_DEV_ERROR(dsi->dev, "dsi controller already in use\n");
> +		mutex_unlock(&dsi->usage_mutex);
> +		return -EBUSY;
> +	}
> +
> +	dsi->usage_mode = DW_DSI_USAGE_DSI;
> +	mutex_unlock(&dsi->usage_mutex);
> +
>   	ret = component_add(dsi->dev, &dw_mipi_dsi_rockchip_ops);
>   	if (ret) {
>   		DRM_DEV_ERROR(dsi->dev, "Failed to register component: %d\n",
> @@ -1000,6 +1035,10 @@ static int dw_mipi_dsi_rockchip_host_detach(void *priv_data,
>   
>   	component_del(dsi->dev, &dw_mipi_dsi_rockchip_ops);
>   
> +	mutex_lock(&dsi->usage_mutex);
> +	dsi->usage_mode = DW_DSI_USAGE_IDLE;
> +	mutex_unlock(&dsi->usage_mutex);
> +
>   	return 0;
>   }
>   
> @@ -1008,11 +1047,227 @@ static const struct dw_mipi_dsi_host_ops dw_mipi_dsi_rockchip_host_ops = {
>   	.detach = dw_mipi_dsi_rockchip_host_detach,
>   };
>   
> +static int dw_mipi_dsi_rockchip_dphy_bind(struct device *dev,
> +					  struct device *master,
> +					  void *data)
> +{
> +	/*
> +	 * Nothing to do when used as a dphy.
> +	 * Just make the rest of Rockchip-DRM happy
> +	 * by being here.
> +	 */
> +
> +	return 0;
> +}
> +
> +static void dw_mipi_dsi_rockchip_dphy_unbind(struct device *dev,
> +					     struct device *master,
> +					     void *data)
> +{
> +	/* Nothing to do when used as a dphy. */
> +}
> +
> +static const struct component_ops dw_mipi_dsi_rockchip_dphy_ops = {
> +	.bind	= dw_mipi_dsi_rockchip_dphy_bind,
> +	.unbind	= dw_mipi_dsi_rockchip_dphy_unbind,
> +};
> +
> +static int dw_mipi_dsi_dphy_init(struct phy *phy)
> +{
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +	int ret;
> +
> +	mutex_lock(&dsi->usage_mutex);
> +
> +	if (dsi->usage_mode != DW_DSI_USAGE_IDLE) {
> +		DRM_DEV_ERROR(dsi->dev, "dsi controller already in use\n");
> +		mutex_unlock(&dsi->usage_mutex);
> +		return -EBUSY;
> +	}
> +
> +	dsi->usage_mode = DW_DSI_USAGE_PHY;
> +	mutex_unlock(&dsi->usage_mutex);
> +
> +	ret = component_add(dsi->dev, &dw_mipi_dsi_rockchip_dphy_ops);
> +	if (ret < 0)
> +		goto err_graph;
> +
> +	if (dsi->cdata->dphy_rx_init) {
> +		ret = clk_prepare_enable(dsi->pclk);
> +		if (ret < 0)
> +			goto err_init;
> +
> +		ret = clk_prepare_enable(dsi->grf_clk);
> +		if (ret) {
> +			clk_disable_unprepare(dsi->pclk);
> +			goto err_init;
> +		}
> +
> +		ret = dsi->cdata->dphy_rx_init(phy);
> +		clk_disable_unprepare(dsi->grf_clk);
> +		clk_disable_unprepare(dsi->pclk);
> +		if (ret < 0)
> +			goto err_init;
> +	}
> +
> +	return 0;
> +
> +err_init:
> +	component_del(dsi->dev, &dw_mipi_dsi_rockchip_dphy_ops);
> +err_graph:
> +	mutex_lock(&dsi->usage_mutex);
> +	dsi->usage_mode = DW_DSI_USAGE_IDLE;
> +	mutex_unlock(&dsi->usage_mutex);
> +
> +	return ret;
> +}
> +
> +static int dw_mipi_dsi_dphy_exit(struct phy *phy)
> +{
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +
> +	component_del(dsi->dev, &dw_mipi_dsi_rockchip_dphy_ops);
> +
> +	mutex_lock(&dsi->usage_mutex);
> +	dsi->usage_mode = DW_DSI_USAGE_IDLE;
> +	mutex_unlock(&dsi->usage_mutex);
> +
> +	return 0;
> +}
> +
> +static int dw_mipi_dsi_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
> +{
> +	struct phy_configure_opts_mipi_dphy *config = &opts->mipi_dphy;
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +	int ret;
> +
> +	ret = phy_mipi_dphy_config_validate(&opts->mipi_dphy);
> +	if (ret)
> +		return ret;
> +
> +	dsi->dphy_config = *config;
> +	dsi->lane_mbps = div_u64(config->hs_clk_rate, 1000 * 1000 * 1);
> +
> +	return 0;
> +}
> +
> +static int dw_mipi_dsi_dphy_power_on(struct phy *phy)
> +{
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +	int i, ret;

It seems "i" could be removed, use ret instead.

In general, the patch doesn't look wrong to me.

For the whole serie:
Acked-by: Helen Koike <helen.koike@collabora.com>

Thanks
Helen

> +
> +	DRM_DEV_DEBUG(dsi->dev, "lanes %d - data_rate_mbps %u\n",
> +		      dsi->dphy_config.lanes, dsi->lane_mbps);
> +
> +	i = max_mbps_to_parameter(dsi->lane_mbps);
> +	if (i < 0) {
> +		DRM_DEV_ERROR(dsi->dev, "failed to get parameter for %dmbps clock\n",
> +			      dsi->lane_mbps);
> +		return i;
> +	}
> +
> +	ret = pm_runtime_get_sync(dsi->dev);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(dsi->dev, "failed to enable device: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(dsi->pclk);
> +	if (ret) {
> +		DRM_DEV_ERROR(dsi->dev, "Failed to enable pclk: %d\n", ret);
> +		goto err_pclk;
> +	}
> +
> +	ret = clk_prepare_enable(dsi->grf_clk);
> +	if (ret) {
> +		DRM_DEV_ERROR(dsi->dev, "Failed to enable grf_clk: %d\n", ret);
> +		goto err_grf_clk;
> +	}
> +
> +	ret = clk_prepare_enable(dsi->phy_cfg_clk);
> +	if (ret) {
> +		DRM_DEV_ERROR(dsi->dev, "Failed to enable phy_cfg_clk: %d\n", ret);
> +		goto err_phy_cfg_clk;
> +	}
> +
> +	/* do soc-variant specific init */
> +	if (dsi->cdata->dphy_rx_power_on) {
> +		ret = dsi->cdata->dphy_rx_power_on(phy);
> +		if (ret < 0) {
> +			DRM_DEV_ERROR(dsi->dev, "hardware-specific phy bringup failed: %d\n", ret);
> +			goto err_pwr_on;
> +		}
> +	}
> +
> +	/*
> +	 * Configure hsfreqrange according to frequency values
> +	 * Set clock lane and hsfreqrange by lane0(test code 0x44)
> +	 */
> +	dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_CLK, 0);
> +	dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_0,
> +			      HSFREQRANGE_SEL(dppa_map[i].hsfreqrange));
> +	dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_1, 0);
> +	dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_2, 0);
> +	dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_3, 0);
> +
> +	/* Normal operation */
> +	dw_mipi_dsi_phy_write(dsi, 0x0, 0);
> +
> +	clk_disable_unprepare(dsi->phy_cfg_clk);
> +	clk_disable_unprepare(dsi->grf_clk);
> +
> +	return ret;
> +
> +err_pwr_on:
> +	clk_disable_unprepare(dsi->phy_cfg_clk);
> +err_phy_cfg_clk:
> +	clk_disable_unprepare(dsi->grf_clk);
> +err_grf_clk:
> +	clk_disable_unprepare(dsi->pclk);
> +err_pclk:
> +	pm_runtime_put(dsi->dev);
> +	return ret;
> +}
> +
> +static int dw_mipi_dsi_dphy_power_off(struct phy *phy)
> +{
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +	int ret;
> +
> +	ret = clk_prepare_enable(dsi->grf_clk);
> +	if (ret) {
> +		DRM_DEV_ERROR(dsi->dev, "Failed to enable grf_clk: %d\n", ret);
> +		return ret;
> +	}
> +
> +	if (dsi->cdata->dphy_rx_power_off) {
> +		ret = dsi->cdata->dphy_rx_power_off(phy);
> +		if (ret < 0)
> +			DRM_DEV_ERROR(dsi->dev, "hardware-specific phy shutdown failed: %d\n", ret);
> +	}
> +
> +	clk_disable_unprepare(dsi->grf_clk);
> +	clk_disable_unprepare(dsi->pclk);
> +
> +	pm_runtime_put(dsi->dev);
> +
> +	return ret;
> +}
> +
> +static const struct phy_ops dw_mipi_dsi_dphy_ops = {
> +	.configure	= dw_mipi_dsi_dphy_configure,
> +	.power_on	= dw_mipi_dsi_dphy_power_on,
> +	.power_off	= dw_mipi_dsi_dphy_power_off,
> +	.init		= dw_mipi_dsi_dphy_init,
> +	.exit		= dw_mipi_dsi_dphy_exit,
> +};
> +
>   static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev)
>   {
>   	struct device *dev = &pdev->dev;
>   	struct device_node *np = dev->of_node;
>   	struct dw_mipi_dsi_rockchip *dsi;
> +	struct phy_provider *phy_provider;
>   	struct resource *res;
>   	const struct rockchip_dw_dsi_chip_data *cdata =
>   				of_device_get_match_data(dev);
> @@ -1109,6 +1364,19 @@ static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev)
>   	dsi->pdata.priv_data = dsi;
>   	platform_set_drvdata(pdev, dsi);
>   
> +	mutex_init(&dsi->usage_mutex);
> +
> +	dsi->dphy = devm_phy_create(dev, NULL, &dw_mipi_dsi_dphy_ops);
> +	if (IS_ERR(dsi->dphy)) {
> +		DRM_DEV_ERROR(&pdev->dev, "failed to create PHY\n");
> +		return PTR_ERR(dsi->dphy);
> +	}
> +
> +	phy_set_drvdata(dsi->dphy, dsi);
> +	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +	if (IS_ERR(phy_provider))
> +		return PTR_ERR(phy_provider);
> +
>   	dsi->dmd = dw_mipi_dsi_probe(pdev, &dsi->pdata);
>   	if (IS_ERR(dsi->dmd)) {
>   		ret = PTR_ERR(dsi->dmd);
> @@ -1175,6 +1443,75 @@ static const struct rockchip_dw_dsi_chip_data rk3288_chip_data[] = {
>   	{ /* sentinel */ }
>   };
>   
> +static int rk3399_dphy_tx1rx1_init(struct phy *phy)
> +{
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +
> +	/*
> +	 * Set TX1RX1 source to isp1.
> +	 * Assume ISP0 is supplied by the RX0 dphy.
> +	 */
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON24,
> +		     HIWORD_UPDATE(0, RK3399_TXRX_SRC_SEL_ISP0));
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON24,
> +		     HIWORD_UPDATE(0, RK3399_TXRX_MASTERSLAVEZ));
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON24,
> +		     HIWORD_UPDATE(0, RK3399_TXRX_BASEDIR));
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON23,
> +		     HIWORD_UPDATE(0, RK3399_DSI1_ENABLE));
> +
> +	return 0;
> +}
> +
> +static int rk3399_dphy_tx1rx1_power_on(struct phy *phy)
> +{
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +
> +	/* tester reset pulse */
> +	dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLK | PHY_TESTCLR);
> +	usleep_range(100, 150);
> +
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON24,
> +		     HIWORD_UPDATE(0, RK3399_TXRX_MASTERSLAVEZ));
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON24,
> +		     HIWORD_UPDATE(RK3399_TXRX_BASEDIR, RK3399_TXRX_BASEDIR));
> +
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON23,
> +		     HIWORD_UPDATE(0, RK3399_DSI1_FORCERXMODE));
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON23,
> +		     HIWORD_UPDATE(0, RK3399_DSI1_FORCETXSTOPMODE));
> +
> +	/* Disable lane turn around, which is ignored in receive mode */
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON24,
> +		     HIWORD_UPDATE(0, RK3399_TXRX_TURNREQUEST));
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON23,
> +		     HIWORD_UPDATE(RK3399_DSI1_TURNDISABLE,
> +				   RK3399_DSI1_TURNDISABLE));
> +	usleep_range(100, 150);
> +
> +	dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLK | PHY_UNTESTCLR);
> +	usleep_range(100, 150);
> +
> +	/* Enable dphy lanes */
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON23,
> +		     HIWORD_UPDATE(GENMASK(dsi->dphy_config.lanes - 1, 0),
> +				   RK3399_DSI1_ENABLE));
> +
> +	usleep_range(100, 150);
> +
> +	return 0;
> +}
> +
> +static int rk3399_dphy_tx1rx1_power_off(struct phy *phy)
> +{
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON23,
> +		     HIWORD_UPDATE(0, RK3399_DSI1_ENABLE));
> +
> +	return 0;
> +}
> +
>   static const struct rockchip_dw_dsi_chip_data rk3399_chip_data[] = {
>   	{
>   		.reg = 0xff960000,
> @@ -1217,6 +1554,10 @@ static const struct rockchip_dw_dsi_chip_data rk3399_chip_data[] = {
>   
>   		.flags = DW_MIPI_NEEDS_PHY_CFG_CLK | DW_MIPI_NEEDS_GRF_CLK,
>   		.max_data_lanes = 4,
> +
> +		.dphy_rx_init = rk3399_dphy_tx1rx1_init,
> +		.dphy_rx_power_on = rk3399_dphy_tx1rx1_power_on,
> +		.dphy_rx_power_off = rk3399_dphy_tx1rx1_power_off,
>   	},
>   	{ /* sentinel */ }
>   };
> 

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <SRS0=4dD8=HR=lists.infradead.org=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@kernel.org>
X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on
	aws-us-west-2-korg-lkml-1.web.codeaurora.org
X-Spam-Level: 
X-Spam-Status: No, score=-15.3 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH,
	DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,
	INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,
	UNPARSEABLE_RELAY,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no
	version=3.4.0
Received: from mail.kernel.org (mail.kernel.org [198.145.29.99])
	by smtp.lore.kernel.org (Postfix) with ESMTP id ACEE1C433DB
	for <linux-arm-kernel@archiver.kernel.org>; Mon, 15 Feb 2021 14:34:50 +0000 (UTC)
Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(No client certificate requested)
	by mail.kernel.org (Postfix) with ESMTPS id 60CDD64E64
	for <linux-arm-kernel@archiver.kernel.org>; Mon, 15 Feb 2021 14:34:50 +0000 (UTC)
DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 60CDD64E64
Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=collabora.com
Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;
	d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Type:
	Content-Transfer-Encoding:Cc:List-Subscribe:List-Help:List-Post:List-Archive:
	List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From:
	References:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date:
	Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner;
	 bh=Qo8w1GodwTlb0jCcKLm8OvmkcM+BuFJG6XSsz48xfvU=; b=SMboTR/Kcm1Jb7BTFAuNvcHVH
	Z7IykwOc+iwJhN2JZwcvpufYCfIoNQefHeBGBW7KgWOcvuM09M5VGKAp7HL5c0eBg3jcWjqn4I4Nl
	fy1OdlnbK5f8k2tRYGz5ibtVA9YYY1IDaAnr6ChWd94mJBnmkH0dyQZV9zxZsHNxXxa5tC7imKKm3
	wjydSWLXod5pt78tE9Lrh83UkdBerDPI6dlNLSW7uqYnuesH09BC+RfmqEMnMS2RDbjOUIbLvBajy
	Hg53smuPnRdDhj6ARaxc/9voVZkQrAqwGCgB+5CIdeHzGFiJi9/Fs1l+GzVMsC2KTPOW/9D/Q23Ol
	vnbxSpjgw==;
Received: from localhost ([::1] helo=merlin.infradead.org)
	by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux))
	id 1lBewS-0005TZ-IX; Mon, 15 Feb 2021 14:33:36 +0000
Received: from bhuna.collabora.co.uk ([46.235.227.227])
 by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux))
 id 1lBewO-0005T5-OE; Mon, 15 Feb 2021 14:33:34 +0000
Received: from [127.0.0.1] (localhost [127.0.0.1])
 (Authenticated sender: koike) with ESMTPSA id C14871F44DE7
Subject: Re: [PATCH 3/6] drm/rockchip: dsi: add ability to work as a phy
 instead of full dsi
To: Heiko Stuebner <heiko@sntech.de>, dri-devel@lists.freedesktop.org
References: <20210210111020.2476369-1-heiko@sntech.de>
 <20210210111020.2476369-4-heiko@sntech.de>
From: Helen Koike <helen.koike@collabora.com>
Message-ID: <d7b8137c-66ce-935a-c8d7-e507146143d7@collabora.com>
Date: Mon, 15 Feb 2021 11:33:19 -0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.7.1
MIME-Version: 1.0
In-Reply-To: <20210210111020.2476369-4-heiko@sntech.de>
Content-Language: en-US
X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 
X-CRM114-CacheID: sfid-20210215_093333_015138_7062A578 
X-CRM114-Status: GOOD (  38.18  )
X-BeenThere: linux-arm-kernel@lists.infradead.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: <linux-arm-kernel.lists.infradead.org>
List-Unsubscribe: <http://lists.infradead.org/mailman/options/linux-arm-kernel>, 
 <mailto:linux-arm-kernel-request@lists.infradead.org?subject=unsubscribe>
List-Archive: <http://lists.infradead.org/pipermail/linux-arm-kernel/>
List-Post: <mailto:linux-arm-kernel@lists.infradead.org>
List-Help: <mailto:linux-arm-kernel-request@lists.infradead.org?subject=help>
List-Subscribe: <http://lists.infradead.org/mailman/listinfo/linux-arm-kernel>, 
 <mailto:linux-arm-kernel-request@lists.infradead.org?subject=subscribe>
Cc: devicetree@vger.kernel.org, dafna.hirschfeld@collabora.com,
 cmuellner@linux.com, hjc@rock-chips.com, linux-kernel@vger.kernel.org,
 linux-rockchip@lists.infradead.org, robh+dt@kernel.org,
 sebastian.fricke@posteo.net,
 Heiko Stuebner <heiko.stuebner@theobroma-systems.com>, ezequiel@collabora.com,
 linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Sender: "linux-arm-kernel" <linux-arm-kernel-bounces@lists.infradead.org>
Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org



On 2/10/21 8:10 AM, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> 
> SoCs like the rk3288 and rk3399 have 3 mipi dphys on them. One is TX-
> only, one is RX-only and one can be configured to do either TX or RX.
> 
> The RX phy is statically connected to the first Image Signal Processor,
> the TX phy is statically connected to the first DSI controller and
> the TXRX phy is connected to both the second DSI controller as well
> as the second ISP.
> 
> The RX dphy is controlled externally through registers in the "General
> Register Files", while the other two are controlled through the
> "Configuration and Test Interface" inside their DSI controller's
> io-memory area.
> 
> The Rockchip dw-dsi controller already controls these dphys for the
> TX case in the driver, but when we want to also allow configuration
> for RX to the ISP from the media subsystem we need to expose phy-
> functionality instead.
> 
> So add a bit of infrastructure to allow the dsi driver to work as a
> phy and make sure it can be only one or the other at a time.
> 
> Similarly as the dsi-controller will be part of the drm-graph when
> active, add an empty component to the drm-graph when in phy-mode
> to make the rest of the drm-graph not wait for it.
> 
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> Tested-by: Sebastian Fricke <sebastian.fricke@posteo.net>
> ---
>   drivers/gpu/drm/rockchip/Kconfig              |   2 +
>   .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c   | 341 ++++++++++++++++++
>   2 files changed, 343 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
> index cb25c0e8fc9b..3094d4533ad6 100644
> --- a/drivers/gpu/drm/rockchip/Kconfig
> +++ b/drivers/gpu/drm/rockchip/Kconfig
> @@ -9,6 +9,8 @@ config DRM_ROCKCHIP
>   	select DRM_ANALOGIX_DP if ROCKCHIP_ANALOGIX_DP
>   	select DRM_DW_HDMI if ROCKCHIP_DW_HDMI
>   	select DRM_DW_MIPI_DSI if ROCKCHIP_DW_MIPI_DSI
> +	select GENERIC_PHY if ROCKCHIP_DW_MIPI_DSI
> +	select GENERIC_PHY_MIPI_DPHY if ROCKCHIP_DW_MIPI_DSI

maybe alphabetical order?

>   	select DRM_RGB if ROCKCHIP_RGB
>   	select SND_SOC_HDMI_CODEC if ROCKCHIP_CDN_DP && SND_SOC
>   	help
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> index 18e112e30f6e..e322749a5279 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> @@ -14,6 +14,7 @@
>   #include <linux/of_device.h>
>   #include <linux/phy/phy.h>
>   #include <linux/pm_runtime.h>
> +#include <linux/phy/phy.h>
>   #include <linux/regmap.h>
>   
>   #include <video/mipi_display.h>
> @@ -125,7 +126,9 @@
>   #define BANDGAP_AND_BIAS_CONTROL			0x20
>   #define TERMINATION_RESISTER_CONTROL			0x21
>   #define AFE_BIAS_BANDGAP_ANALOG_PROGRAMMABILITY		0x22
> +#define HS_RX_CONTROL_OF_LANE_CLK			0x34
>   #define HS_RX_CONTROL_OF_LANE_0				0x44
> +#define HS_RX_CONTROL_OF_LANE_1				0x54
>   #define HS_TX_CLOCK_LANE_REQUEST_STATE_TIME_CONTROL	0x60
>   #define HS_TX_CLOCK_LANE_PREPARE_STATE_TIME_CONTROL	0x61
>   #define HS_TX_CLOCK_LANE_HS_ZERO_STATE_TIME_CONTROL	0x62
> @@ -137,6 +140,9 @@
>   #define HS_TX_DATA_LANE_HS_ZERO_STATE_TIME_CONTROL	0x72
>   #define HS_TX_DATA_LANE_TRAIL_STATE_TIME_CONTROL	0x73
>   #define HS_TX_DATA_LANE_EXIT_STATE_TIME_CONTROL		0x74
> +#define HS_RX_DATA_LANE_THS_SETTLE_CONTROL		0x75
> +#define HS_RX_CONTROL_OF_LANE_2				0x84
> +#define HS_RX_CONTROL_OF_LANE_3				0x94
>   
>   #define DW_MIPI_NEEDS_PHY_CFG_CLK	BIT(0)
>   #define DW_MIPI_NEEDS_GRF_CLK		BIT(1)
> @@ -171,11 +177,19 @@
>   #define RK3399_TXRX_MASTERSLAVEZ	BIT(7)
>   #define RK3399_TXRX_ENABLECLK		BIT(6)
>   #define RK3399_TXRX_BASEDIR		BIT(5)
> +#define RK3399_TXRX_SRC_SEL_ISP0	BIT(4)
> +#define RK3399_TXRX_TURNREQUEST		GENMASK(3, 0)
>   
>   #define HIWORD_UPDATE(val, mask)	(val | (mask) << 16)
>   
>   #define to_dsi(nm)	container_of(nm, struct dw_mipi_dsi_rockchip, nm)
>   
> +enum {
> +	DW_DSI_USAGE_IDLE,
> +	DW_DSI_USAGE_DSI,
> +	DW_DSI_USAGE_PHY,
> +};
> +
>   enum {
>   	BANDGAP_97_07,
>   	BANDGAP_98_05,
> @@ -213,6 +227,10 @@ struct rockchip_dw_dsi_chip_data {
>   	u32 lanecfg2_grf_reg;
>   	u32 lanecfg2;
>   
> +	int (*dphy_rx_init)(struct phy *phy);
> +	int (*dphy_rx_power_on)(struct phy *phy);
> +	int (*dphy_rx_power_off)(struct phy *phy);
> +
>   	unsigned int flags;
>   	unsigned int max_data_lanes;
>   };
> @@ -236,6 +254,12 @@ struct dw_mipi_dsi_rockchip {
>   	struct phy *phy;
>   	union phy_configure_opts phy_opts;
>   
> +	/* being a phy for other mipi hosts */
> +	unsigned int usage_mode;
> +	struct mutex usage_mutex;
> +	struct phy *dphy;
> +	struct phy_configure_opts_mipi_dphy dphy_config;
> +
>   	unsigned int lane_mbps; /* per lane */
>   	u16 input_div;
>   	u16 feedback_div;
> @@ -965,6 +989,17 @@ static int dw_mipi_dsi_rockchip_host_attach(void *priv_data,
>   	struct device *second;
>   	int ret;
>   
> +	mutex_lock(&dsi->usage_mutex);
> +
> +	if (dsi->usage_mode != DW_DSI_USAGE_IDLE) {
> +		DRM_DEV_ERROR(dsi->dev, "dsi controller already in use\n");
> +		mutex_unlock(&dsi->usage_mutex);
> +		return -EBUSY;
> +	}
> +
> +	dsi->usage_mode = DW_DSI_USAGE_DSI;
> +	mutex_unlock(&dsi->usage_mutex);
> +
>   	ret = component_add(dsi->dev, &dw_mipi_dsi_rockchip_ops);
>   	if (ret) {
>   		DRM_DEV_ERROR(dsi->dev, "Failed to register component: %d\n",
> @@ -1000,6 +1035,10 @@ static int dw_mipi_dsi_rockchip_host_detach(void *priv_data,
>   
>   	component_del(dsi->dev, &dw_mipi_dsi_rockchip_ops);
>   
> +	mutex_lock(&dsi->usage_mutex);
> +	dsi->usage_mode = DW_DSI_USAGE_IDLE;
> +	mutex_unlock(&dsi->usage_mutex);
> +
>   	return 0;
>   }
>   
> @@ -1008,11 +1047,227 @@ static const struct dw_mipi_dsi_host_ops dw_mipi_dsi_rockchip_host_ops = {
>   	.detach = dw_mipi_dsi_rockchip_host_detach,
>   };
>   
> +static int dw_mipi_dsi_rockchip_dphy_bind(struct device *dev,
> +					  struct device *master,
> +					  void *data)
> +{
> +	/*
> +	 * Nothing to do when used as a dphy.
> +	 * Just make the rest of Rockchip-DRM happy
> +	 * by being here.
> +	 */
> +
> +	return 0;
> +}
> +
> +static void dw_mipi_dsi_rockchip_dphy_unbind(struct device *dev,
> +					     struct device *master,
> +					     void *data)
> +{
> +	/* Nothing to do when used as a dphy. */
> +}
> +
> +static const struct component_ops dw_mipi_dsi_rockchip_dphy_ops = {
> +	.bind	= dw_mipi_dsi_rockchip_dphy_bind,
> +	.unbind	= dw_mipi_dsi_rockchip_dphy_unbind,
> +};
> +
> +static int dw_mipi_dsi_dphy_init(struct phy *phy)
> +{
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +	int ret;
> +
> +	mutex_lock(&dsi->usage_mutex);
> +
> +	if (dsi->usage_mode != DW_DSI_USAGE_IDLE) {
> +		DRM_DEV_ERROR(dsi->dev, "dsi controller already in use\n");
> +		mutex_unlock(&dsi->usage_mutex);
> +		return -EBUSY;
> +	}
> +
> +	dsi->usage_mode = DW_DSI_USAGE_PHY;
> +	mutex_unlock(&dsi->usage_mutex);
> +
> +	ret = component_add(dsi->dev, &dw_mipi_dsi_rockchip_dphy_ops);
> +	if (ret < 0)
> +		goto err_graph;
> +
> +	if (dsi->cdata->dphy_rx_init) {
> +		ret = clk_prepare_enable(dsi->pclk);
> +		if (ret < 0)
> +			goto err_init;
> +
> +		ret = clk_prepare_enable(dsi->grf_clk);
> +		if (ret) {
> +			clk_disable_unprepare(dsi->pclk);
> +			goto err_init;
> +		}
> +
> +		ret = dsi->cdata->dphy_rx_init(phy);
> +		clk_disable_unprepare(dsi->grf_clk);
> +		clk_disable_unprepare(dsi->pclk);
> +		if (ret < 0)
> +			goto err_init;
> +	}
> +
> +	return 0;
> +
> +err_init:
> +	component_del(dsi->dev, &dw_mipi_dsi_rockchip_dphy_ops);
> +err_graph:
> +	mutex_lock(&dsi->usage_mutex);
> +	dsi->usage_mode = DW_DSI_USAGE_IDLE;
> +	mutex_unlock(&dsi->usage_mutex);
> +
> +	return ret;
> +}
> +
> +static int dw_mipi_dsi_dphy_exit(struct phy *phy)
> +{
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +
> +	component_del(dsi->dev, &dw_mipi_dsi_rockchip_dphy_ops);
> +
> +	mutex_lock(&dsi->usage_mutex);
> +	dsi->usage_mode = DW_DSI_USAGE_IDLE;
> +	mutex_unlock(&dsi->usage_mutex);
> +
> +	return 0;
> +}
> +
> +static int dw_mipi_dsi_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
> +{
> +	struct phy_configure_opts_mipi_dphy *config = &opts->mipi_dphy;
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +	int ret;
> +
> +	ret = phy_mipi_dphy_config_validate(&opts->mipi_dphy);
> +	if (ret)
> +		return ret;
> +
> +	dsi->dphy_config = *config;
> +	dsi->lane_mbps = div_u64(config->hs_clk_rate, 1000 * 1000 * 1);
> +
> +	return 0;
> +}
> +
> +static int dw_mipi_dsi_dphy_power_on(struct phy *phy)
> +{
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +	int i, ret;

It seems "i" could be removed, use ret instead.

In general, the patch doesn't look wrong to me.

For the whole serie:
Acked-by: Helen Koike <helen.koike@collabora.com>

Thanks
Helen

> +
> +	DRM_DEV_DEBUG(dsi->dev, "lanes %d - data_rate_mbps %u\n",
> +		      dsi->dphy_config.lanes, dsi->lane_mbps);
> +
> +	i = max_mbps_to_parameter(dsi->lane_mbps);
> +	if (i < 0) {
> +		DRM_DEV_ERROR(dsi->dev, "failed to get parameter for %dmbps clock\n",
> +			      dsi->lane_mbps);
> +		return i;
> +	}
> +
> +	ret = pm_runtime_get_sync(dsi->dev);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(dsi->dev, "failed to enable device: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(dsi->pclk);
> +	if (ret) {
> +		DRM_DEV_ERROR(dsi->dev, "Failed to enable pclk: %d\n", ret);
> +		goto err_pclk;
> +	}
> +
> +	ret = clk_prepare_enable(dsi->grf_clk);
> +	if (ret) {
> +		DRM_DEV_ERROR(dsi->dev, "Failed to enable grf_clk: %d\n", ret);
> +		goto err_grf_clk;
> +	}
> +
> +	ret = clk_prepare_enable(dsi->phy_cfg_clk);
> +	if (ret) {
> +		DRM_DEV_ERROR(dsi->dev, "Failed to enable phy_cfg_clk: %d\n", ret);
> +		goto err_phy_cfg_clk;
> +	}
> +
> +	/* do soc-variant specific init */
> +	if (dsi->cdata->dphy_rx_power_on) {
> +		ret = dsi->cdata->dphy_rx_power_on(phy);
> +		if (ret < 0) {
> +			DRM_DEV_ERROR(dsi->dev, "hardware-specific phy bringup failed: %d\n", ret);
> +			goto err_pwr_on;
> +		}
> +	}
> +
> +	/*
> +	 * Configure hsfreqrange according to frequency values
> +	 * Set clock lane and hsfreqrange by lane0(test code 0x44)
> +	 */
> +	dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_CLK, 0);
> +	dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_0,
> +			      HSFREQRANGE_SEL(dppa_map[i].hsfreqrange));
> +	dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_1, 0);
> +	dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_2, 0);
> +	dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_3, 0);
> +
> +	/* Normal operation */
> +	dw_mipi_dsi_phy_write(dsi, 0x0, 0);
> +
> +	clk_disable_unprepare(dsi->phy_cfg_clk);
> +	clk_disable_unprepare(dsi->grf_clk);
> +
> +	return ret;
> +
> +err_pwr_on:
> +	clk_disable_unprepare(dsi->phy_cfg_clk);
> +err_phy_cfg_clk:
> +	clk_disable_unprepare(dsi->grf_clk);
> +err_grf_clk:
> +	clk_disable_unprepare(dsi->pclk);
> +err_pclk:
> +	pm_runtime_put(dsi->dev);
> +	return ret;
> +}
> +
> +static int dw_mipi_dsi_dphy_power_off(struct phy *phy)
> +{
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +	int ret;
> +
> +	ret = clk_prepare_enable(dsi->grf_clk);
> +	if (ret) {
> +		DRM_DEV_ERROR(dsi->dev, "Failed to enable grf_clk: %d\n", ret);
> +		return ret;
> +	}
> +
> +	if (dsi->cdata->dphy_rx_power_off) {
> +		ret = dsi->cdata->dphy_rx_power_off(phy);
> +		if (ret < 0)
> +			DRM_DEV_ERROR(dsi->dev, "hardware-specific phy shutdown failed: %d\n", ret);
> +	}
> +
> +	clk_disable_unprepare(dsi->grf_clk);
> +	clk_disable_unprepare(dsi->pclk);
> +
> +	pm_runtime_put(dsi->dev);
> +
> +	return ret;
> +}
> +
> +static const struct phy_ops dw_mipi_dsi_dphy_ops = {
> +	.configure	= dw_mipi_dsi_dphy_configure,
> +	.power_on	= dw_mipi_dsi_dphy_power_on,
> +	.power_off	= dw_mipi_dsi_dphy_power_off,
> +	.init		= dw_mipi_dsi_dphy_init,
> +	.exit		= dw_mipi_dsi_dphy_exit,
> +};
> +
>   static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev)
>   {
>   	struct device *dev = &pdev->dev;
>   	struct device_node *np = dev->of_node;
>   	struct dw_mipi_dsi_rockchip *dsi;
> +	struct phy_provider *phy_provider;
>   	struct resource *res;
>   	const struct rockchip_dw_dsi_chip_data *cdata =
>   				of_device_get_match_data(dev);
> @@ -1109,6 +1364,19 @@ static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev)
>   	dsi->pdata.priv_data = dsi;
>   	platform_set_drvdata(pdev, dsi);
>   
> +	mutex_init(&dsi->usage_mutex);
> +
> +	dsi->dphy = devm_phy_create(dev, NULL, &dw_mipi_dsi_dphy_ops);
> +	if (IS_ERR(dsi->dphy)) {
> +		DRM_DEV_ERROR(&pdev->dev, "failed to create PHY\n");
> +		return PTR_ERR(dsi->dphy);
> +	}
> +
> +	phy_set_drvdata(dsi->dphy, dsi);
> +	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +	if (IS_ERR(phy_provider))
> +		return PTR_ERR(phy_provider);
> +
>   	dsi->dmd = dw_mipi_dsi_probe(pdev, &dsi->pdata);
>   	if (IS_ERR(dsi->dmd)) {
>   		ret = PTR_ERR(dsi->dmd);
> @@ -1175,6 +1443,75 @@ static const struct rockchip_dw_dsi_chip_data rk3288_chip_data[] = {
>   	{ /* sentinel */ }
>   };
>   
> +static int rk3399_dphy_tx1rx1_init(struct phy *phy)
> +{
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +
> +	/*
> +	 * Set TX1RX1 source to isp1.
> +	 * Assume ISP0 is supplied by the RX0 dphy.
> +	 */
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON24,
> +		     HIWORD_UPDATE(0, RK3399_TXRX_SRC_SEL_ISP0));
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON24,
> +		     HIWORD_UPDATE(0, RK3399_TXRX_MASTERSLAVEZ));
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON24,
> +		     HIWORD_UPDATE(0, RK3399_TXRX_BASEDIR));
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON23,
> +		     HIWORD_UPDATE(0, RK3399_DSI1_ENABLE));
> +
> +	return 0;
> +}
> +
> +static int rk3399_dphy_tx1rx1_power_on(struct phy *phy)
> +{
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +
> +	/* tester reset pulse */
> +	dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLK | PHY_TESTCLR);
> +	usleep_range(100, 150);
> +
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON24,
> +		     HIWORD_UPDATE(0, RK3399_TXRX_MASTERSLAVEZ));
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON24,
> +		     HIWORD_UPDATE(RK3399_TXRX_BASEDIR, RK3399_TXRX_BASEDIR));
> +
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON23,
> +		     HIWORD_UPDATE(0, RK3399_DSI1_FORCERXMODE));
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON23,
> +		     HIWORD_UPDATE(0, RK3399_DSI1_FORCETXSTOPMODE));
> +
> +	/* Disable lane turn around, which is ignored in receive mode */
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON24,
> +		     HIWORD_UPDATE(0, RK3399_TXRX_TURNREQUEST));
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON23,
> +		     HIWORD_UPDATE(RK3399_DSI1_TURNDISABLE,
> +				   RK3399_DSI1_TURNDISABLE));
> +	usleep_range(100, 150);
> +
> +	dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLK | PHY_UNTESTCLR);
> +	usleep_range(100, 150);
> +
> +	/* Enable dphy lanes */
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON23,
> +		     HIWORD_UPDATE(GENMASK(dsi->dphy_config.lanes - 1, 0),
> +				   RK3399_DSI1_ENABLE));
> +
> +	usleep_range(100, 150);
> +
> +	return 0;
> +}
> +
> +static int rk3399_dphy_tx1rx1_power_off(struct phy *phy)
> +{
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON23,
> +		     HIWORD_UPDATE(0, RK3399_DSI1_ENABLE));
> +
> +	return 0;
> +}
> +
>   static const struct rockchip_dw_dsi_chip_data rk3399_chip_data[] = {
>   	{
>   		.reg = 0xff960000,
> @@ -1217,6 +1554,10 @@ static const struct rockchip_dw_dsi_chip_data rk3399_chip_data[] = {
>   
>   		.flags = DW_MIPI_NEEDS_PHY_CFG_CLK | DW_MIPI_NEEDS_GRF_CLK,
>   		.max_data_lanes = 4,
> +
> +		.dphy_rx_init = rk3399_dphy_tx1rx1_init,
> +		.dphy_rx_power_on = rk3399_dphy_tx1rx1_power_on,
> +		.dphy_rx_power_off = rk3399_dphy_tx1rx1_power_off,
>   	},
>   	{ /* sentinel */ }
>   };
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <SRS0=V4+1=HR=lists.freedesktop.org=dri-devel-bounces@kernel.org>
X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on
	aws-us-west-2-korg-lkml-1.web.codeaurora.org
X-Spam-Level: 
X-Spam-Status: No, score=-15.3 required=3.0 tests=BAYES_00,
	HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH,
	MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY,
	USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0
Received: from mail.kernel.org (mail.kernel.org [198.145.29.99])
	by smtp.lore.kernel.org (Postfix) with ESMTP id C39ACC433E0
	for <dri-devel@archiver.kernel.org>; Mon, 15 Feb 2021 14:36:46 +0000 (UTC)
Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(No client certificate requested)
	by mail.kernel.org (Postfix) with ESMTPS id 7ABC760C3D
	for <dri-devel@archiver.kernel.org>; Mon, 15 Feb 2021 14:36:46 +0000 (UTC)
DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7ABC760C3D
Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=collabora.com
Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org
Received: from gabe.freedesktop.org (localhost [127.0.0.1])
	by gabe.freedesktop.org (Postfix) with ESMTP id 0550F6E29D;
	Mon, 15 Feb 2021 14:36:46 +0000 (UTC)
Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk
 [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3])
 by gabe.freedesktop.org (Postfix) with ESMTPS id B08076E29D
 for <dri-devel@lists.freedesktop.org>; Mon, 15 Feb 2021 14:33:30 +0000 (UTC)
Received: from [127.0.0.1] (localhost [127.0.0.1])
 (Authenticated sender: koike) with ESMTPSA id C14871F44DE7
Subject: Re: [PATCH 3/6] drm/rockchip: dsi: add ability to work as a phy
 instead of full dsi
To: Heiko Stuebner <heiko@sntech.de>, dri-devel@lists.freedesktop.org
References: <20210210111020.2476369-1-heiko@sntech.de>
 <20210210111020.2476369-4-heiko@sntech.de>
From: Helen Koike <helen.koike@collabora.com>
Message-ID: <d7b8137c-66ce-935a-c8d7-e507146143d7@collabora.com>
Date: Mon, 15 Feb 2021 11:33:19 -0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.7.1
MIME-Version: 1.0
In-Reply-To: <20210210111020.2476369-4-heiko@sntech.de>
Content-Language: en-US
X-Mailman-Approved-At: Mon, 15 Feb 2021 14:36:44 +0000
X-BeenThere: dri-devel@lists.freedesktop.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Direct Rendering Infrastructure - Development
 <dri-devel.lists.freedesktop.org>
List-Unsubscribe: <https://lists.freedesktop.org/mailman/options/dri-devel>,
 <mailto:dri-devel-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <https://lists.freedesktop.org/archives/dri-devel>
List-Post: <mailto:dri-devel@lists.freedesktop.org>
List-Help: <mailto:dri-devel-request@lists.freedesktop.org?subject=help>
List-Subscribe: <https://lists.freedesktop.org/mailman/listinfo/dri-devel>,
 <mailto:dri-devel-request@lists.freedesktop.org?subject=subscribe>
Cc: devicetree@vger.kernel.org, dafna.hirschfeld@collabora.com,
 cmuellner@linux.com, hjc@rock-chips.com, linux-kernel@vger.kernel.org,
 linux-rockchip@lists.infradead.org, robh+dt@kernel.org,
 sebastian.fricke@posteo.net,
 Heiko Stuebner <heiko.stuebner@theobroma-systems.com>, ezequiel@collabora.com,
 linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Errors-To: dri-devel-bounces@lists.freedesktop.org
Sender: "dri-devel" <dri-devel-bounces@lists.freedesktop.org>



On 2/10/21 8:10 AM, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> 
> SoCs like the rk3288 and rk3399 have 3 mipi dphys on them. One is TX-
> only, one is RX-only and one can be configured to do either TX or RX.
> 
> The RX phy is statically connected to the first Image Signal Processor,
> the TX phy is statically connected to the first DSI controller and
> the TXRX phy is connected to both the second DSI controller as well
> as the second ISP.
> 
> The RX dphy is controlled externally through registers in the "General
> Register Files", while the other two are controlled through the
> "Configuration and Test Interface" inside their DSI controller's
> io-memory area.
> 
> The Rockchip dw-dsi controller already controls these dphys for the
> TX case in the driver, but when we want to also allow configuration
> for RX to the ISP from the media subsystem we need to expose phy-
> functionality instead.
> 
> So add a bit of infrastructure to allow the dsi driver to work as a
> phy and make sure it can be only one or the other at a time.
> 
> Similarly as the dsi-controller will be part of the drm-graph when
> active, add an empty component to the drm-graph when in phy-mode
> to make the rest of the drm-graph not wait for it.
> 
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> Tested-by: Sebastian Fricke <sebastian.fricke@posteo.net>
> ---
>   drivers/gpu/drm/rockchip/Kconfig              |   2 +
>   .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c   | 341 ++++++++++++++++++
>   2 files changed, 343 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
> index cb25c0e8fc9b..3094d4533ad6 100644
> --- a/drivers/gpu/drm/rockchip/Kconfig
> +++ b/drivers/gpu/drm/rockchip/Kconfig
> @@ -9,6 +9,8 @@ config DRM_ROCKCHIP
>   	select DRM_ANALOGIX_DP if ROCKCHIP_ANALOGIX_DP
>   	select DRM_DW_HDMI if ROCKCHIP_DW_HDMI
>   	select DRM_DW_MIPI_DSI if ROCKCHIP_DW_MIPI_DSI
> +	select GENERIC_PHY if ROCKCHIP_DW_MIPI_DSI
> +	select GENERIC_PHY_MIPI_DPHY if ROCKCHIP_DW_MIPI_DSI

maybe alphabetical order?

>   	select DRM_RGB if ROCKCHIP_RGB
>   	select SND_SOC_HDMI_CODEC if ROCKCHIP_CDN_DP && SND_SOC
>   	help
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> index 18e112e30f6e..e322749a5279 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> @@ -14,6 +14,7 @@
>   #include <linux/of_device.h>
>   #include <linux/phy/phy.h>
>   #include <linux/pm_runtime.h>
> +#include <linux/phy/phy.h>
>   #include <linux/regmap.h>
>   
>   #include <video/mipi_display.h>
> @@ -125,7 +126,9 @@
>   #define BANDGAP_AND_BIAS_CONTROL			0x20
>   #define TERMINATION_RESISTER_CONTROL			0x21
>   #define AFE_BIAS_BANDGAP_ANALOG_PROGRAMMABILITY		0x22
> +#define HS_RX_CONTROL_OF_LANE_CLK			0x34
>   #define HS_RX_CONTROL_OF_LANE_0				0x44
> +#define HS_RX_CONTROL_OF_LANE_1				0x54
>   #define HS_TX_CLOCK_LANE_REQUEST_STATE_TIME_CONTROL	0x60
>   #define HS_TX_CLOCK_LANE_PREPARE_STATE_TIME_CONTROL	0x61
>   #define HS_TX_CLOCK_LANE_HS_ZERO_STATE_TIME_CONTROL	0x62
> @@ -137,6 +140,9 @@
>   #define HS_TX_DATA_LANE_HS_ZERO_STATE_TIME_CONTROL	0x72
>   #define HS_TX_DATA_LANE_TRAIL_STATE_TIME_CONTROL	0x73
>   #define HS_TX_DATA_LANE_EXIT_STATE_TIME_CONTROL		0x74
> +#define HS_RX_DATA_LANE_THS_SETTLE_CONTROL		0x75
> +#define HS_RX_CONTROL_OF_LANE_2				0x84
> +#define HS_RX_CONTROL_OF_LANE_3				0x94
>   
>   #define DW_MIPI_NEEDS_PHY_CFG_CLK	BIT(0)
>   #define DW_MIPI_NEEDS_GRF_CLK		BIT(1)
> @@ -171,11 +177,19 @@
>   #define RK3399_TXRX_MASTERSLAVEZ	BIT(7)
>   #define RK3399_TXRX_ENABLECLK		BIT(6)
>   #define RK3399_TXRX_BASEDIR		BIT(5)
> +#define RK3399_TXRX_SRC_SEL_ISP0	BIT(4)
> +#define RK3399_TXRX_TURNREQUEST		GENMASK(3, 0)
>   
>   #define HIWORD_UPDATE(val, mask)	(val | (mask) << 16)
>   
>   #define to_dsi(nm)	container_of(nm, struct dw_mipi_dsi_rockchip, nm)
>   
> +enum {
> +	DW_DSI_USAGE_IDLE,
> +	DW_DSI_USAGE_DSI,
> +	DW_DSI_USAGE_PHY,
> +};
> +
>   enum {
>   	BANDGAP_97_07,
>   	BANDGAP_98_05,
> @@ -213,6 +227,10 @@ struct rockchip_dw_dsi_chip_data {
>   	u32 lanecfg2_grf_reg;
>   	u32 lanecfg2;
>   
> +	int (*dphy_rx_init)(struct phy *phy);
> +	int (*dphy_rx_power_on)(struct phy *phy);
> +	int (*dphy_rx_power_off)(struct phy *phy);
> +
>   	unsigned int flags;
>   	unsigned int max_data_lanes;
>   };
> @@ -236,6 +254,12 @@ struct dw_mipi_dsi_rockchip {
>   	struct phy *phy;
>   	union phy_configure_opts phy_opts;
>   
> +	/* being a phy for other mipi hosts */
> +	unsigned int usage_mode;
> +	struct mutex usage_mutex;
> +	struct phy *dphy;
> +	struct phy_configure_opts_mipi_dphy dphy_config;
> +
>   	unsigned int lane_mbps; /* per lane */
>   	u16 input_div;
>   	u16 feedback_div;
> @@ -965,6 +989,17 @@ static int dw_mipi_dsi_rockchip_host_attach(void *priv_data,
>   	struct device *second;
>   	int ret;
>   
> +	mutex_lock(&dsi->usage_mutex);
> +
> +	if (dsi->usage_mode != DW_DSI_USAGE_IDLE) {
> +		DRM_DEV_ERROR(dsi->dev, "dsi controller already in use\n");
> +		mutex_unlock(&dsi->usage_mutex);
> +		return -EBUSY;
> +	}
> +
> +	dsi->usage_mode = DW_DSI_USAGE_DSI;
> +	mutex_unlock(&dsi->usage_mutex);
> +
>   	ret = component_add(dsi->dev, &dw_mipi_dsi_rockchip_ops);
>   	if (ret) {
>   		DRM_DEV_ERROR(dsi->dev, "Failed to register component: %d\n",
> @@ -1000,6 +1035,10 @@ static int dw_mipi_dsi_rockchip_host_detach(void *priv_data,
>   
>   	component_del(dsi->dev, &dw_mipi_dsi_rockchip_ops);
>   
> +	mutex_lock(&dsi->usage_mutex);
> +	dsi->usage_mode = DW_DSI_USAGE_IDLE;
> +	mutex_unlock(&dsi->usage_mutex);
> +
>   	return 0;
>   }
>   
> @@ -1008,11 +1047,227 @@ static const struct dw_mipi_dsi_host_ops dw_mipi_dsi_rockchip_host_ops = {
>   	.detach = dw_mipi_dsi_rockchip_host_detach,
>   };
>   
> +static int dw_mipi_dsi_rockchip_dphy_bind(struct device *dev,
> +					  struct device *master,
> +					  void *data)
> +{
> +	/*
> +	 * Nothing to do when used as a dphy.
> +	 * Just make the rest of Rockchip-DRM happy
> +	 * by being here.
> +	 */
> +
> +	return 0;
> +}
> +
> +static void dw_mipi_dsi_rockchip_dphy_unbind(struct device *dev,
> +					     struct device *master,
> +					     void *data)
> +{
> +	/* Nothing to do when used as a dphy. */
> +}
> +
> +static const struct component_ops dw_mipi_dsi_rockchip_dphy_ops = {
> +	.bind	= dw_mipi_dsi_rockchip_dphy_bind,
> +	.unbind	= dw_mipi_dsi_rockchip_dphy_unbind,
> +};
> +
> +static int dw_mipi_dsi_dphy_init(struct phy *phy)
> +{
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +	int ret;
> +
> +	mutex_lock(&dsi->usage_mutex);
> +
> +	if (dsi->usage_mode != DW_DSI_USAGE_IDLE) {
> +		DRM_DEV_ERROR(dsi->dev, "dsi controller already in use\n");
> +		mutex_unlock(&dsi->usage_mutex);
> +		return -EBUSY;
> +	}
> +
> +	dsi->usage_mode = DW_DSI_USAGE_PHY;
> +	mutex_unlock(&dsi->usage_mutex);
> +
> +	ret = component_add(dsi->dev, &dw_mipi_dsi_rockchip_dphy_ops);
> +	if (ret < 0)
> +		goto err_graph;
> +
> +	if (dsi->cdata->dphy_rx_init) {
> +		ret = clk_prepare_enable(dsi->pclk);
> +		if (ret < 0)
> +			goto err_init;
> +
> +		ret = clk_prepare_enable(dsi->grf_clk);
> +		if (ret) {
> +			clk_disable_unprepare(dsi->pclk);
> +			goto err_init;
> +		}
> +
> +		ret = dsi->cdata->dphy_rx_init(phy);
> +		clk_disable_unprepare(dsi->grf_clk);
> +		clk_disable_unprepare(dsi->pclk);
> +		if (ret < 0)
> +			goto err_init;
> +	}
> +
> +	return 0;
> +
> +err_init:
> +	component_del(dsi->dev, &dw_mipi_dsi_rockchip_dphy_ops);
> +err_graph:
> +	mutex_lock(&dsi->usage_mutex);
> +	dsi->usage_mode = DW_DSI_USAGE_IDLE;
> +	mutex_unlock(&dsi->usage_mutex);
> +
> +	return ret;
> +}
> +
> +static int dw_mipi_dsi_dphy_exit(struct phy *phy)
> +{
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +
> +	component_del(dsi->dev, &dw_mipi_dsi_rockchip_dphy_ops);
> +
> +	mutex_lock(&dsi->usage_mutex);
> +	dsi->usage_mode = DW_DSI_USAGE_IDLE;
> +	mutex_unlock(&dsi->usage_mutex);
> +
> +	return 0;
> +}
> +
> +static int dw_mipi_dsi_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
> +{
> +	struct phy_configure_opts_mipi_dphy *config = &opts->mipi_dphy;
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +	int ret;
> +
> +	ret = phy_mipi_dphy_config_validate(&opts->mipi_dphy);
> +	if (ret)
> +		return ret;
> +
> +	dsi->dphy_config = *config;
> +	dsi->lane_mbps = div_u64(config->hs_clk_rate, 1000 * 1000 * 1);
> +
> +	return 0;
> +}
> +
> +static int dw_mipi_dsi_dphy_power_on(struct phy *phy)
> +{
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +	int i, ret;

It seems "i" could be removed, use ret instead.

In general, the patch doesn't look wrong to me.

For the whole serie:
Acked-by: Helen Koike <helen.koike@collabora.com>

Thanks
Helen

> +
> +	DRM_DEV_DEBUG(dsi->dev, "lanes %d - data_rate_mbps %u\n",
> +		      dsi->dphy_config.lanes, dsi->lane_mbps);
> +
> +	i = max_mbps_to_parameter(dsi->lane_mbps);
> +	if (i < 0) {
> +		DRM_DEV_ERROR(dsi->dev, "failed to get parameter for %dmbps clock\n",
> +			      dsi->lane_mbps);
> +		return i;
> +	}
> +
> +	ret = pm_runtime_get_sync(dsi->dev);
> +	if (ret < 0) {
> +		DRM_DEV_ERROR(dsi->dev, "failed to enable device: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(dsi->pclk);
> +	if (ret) {
> +		DRM_DEV_ERROR(dsi->dev, "Failed to enable pclk: %d\n", ret);
> +		goto err_pclk;
> +	}
> +
> +	ret = clk_prepare_enable(dsi->grf_clk);
> +	if (ret) {
> +		DRM_DEV_ERROR(dsi->dev, "Failed to enable grf_clk: %d\n", ret);
> +		goto err_grf_clk;
> +	}
> +
> +	ret = clk_prepare_enable(dsi->phy_cfg_clk);
> +	if (ret) {
> +		DRM_DEV_ERROR(dsi->dev, "Failed to enable phy_cfg_clk: %d\n", ret);
> +		goto err_phy_cfg_clk;
> +	}
> +
> +	/* do soc-variant specific init */
> +	if (dsi->cdata->dphy_rx_power_on) {
> +		ret = dsi->cdata->dphy_rx_power_on(phy);
> +		if (ret < 0) {
> +			DRM_DEV_ERROR(dsi->dev, "hardware-specific phy bringup failed: %d\n", ret);
> +			goto err_pwr_on;
> +		}
> +	}
> +
> +	/*
> +	 * Configure hsfreqrange according to frequency values
> +	 * Set clock lane and hsfreqrange by lane0(test code 0x44)
> +	 */
> +	dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_CLK, 0);
> +	dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_0,
> +			      HSFREQRANGE_SEL(dppa_map[i].hsfreqrange));
> +	dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_1, 0);
> +	dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_2, 0);
> +	dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_3, 0);
> +
> +	/* Normal operation */
> +	dw_mipi_dsi_phy_write(dsi, 0x0, 0);
> +
> +	clk_disable_unprepare(dsi->phy_cfg_clk);
> +	clk_disable_unprepare(dsi->grf_clk);
> +
> +	return ret;
> +
> +err_pwr_on:
> +	clk_disable_unprepare(dsi->phy_cfg_clk);
> +err_phy_cfg_clk:
> +	clk_disable_unprepare(dsi->grf_clk);
> +err_grf_clk:
> +	clk_disable_unprepare(dsi->pclk);
> +err_pclk:
> +	pm_runtime_put(dsi->dev);
> +	return ret;
> +}
> +
> +static int dw_mipi_dsi_dphy_power_off(struct phy *phy)
> +{
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +	int ret;
> +
> +	ret = clk_prepare_enable(dsi->grf_clk);
> +	if (ret) {
> +		DRM_DEV_ERROR(dsi->dev, "Failed to enable grf_clk: %d\n", ret);
> +		return ret;
> +	}
> +
> +	if (dsi->cdata->dphy_rx_power_off) {
> +		ret = dsi->cdata->dphy_rx_power_off(phy);
> +		if (ret < 0)
> +			DRM_DEV_ERROR(dsi->dev, "hardware-specific phy shutdown failed: %d\n", ret);
> +	}
> +
> +	clk_disable_unprepare(dsi->grf_clk);
> +	clk_disable_unprepare(dsi->pclk);
> +
> +	pm_runtime_put(dsi->dev);
> +
> +	return ret;
> +}
> +
> +static const struct phy_ops dw_mipi_dsi_dphy_ops = {
> +	.configure	= dw_mipi_dsi_dphy_configure,
> +	.power_on	= dw_mipi_dsi_dphy_power_on,
> +	.power_off	= dw_mipi_dsi_dphy_power_off,
> +	.init		= dw_mipi_dsi_dphy_init,
> +	.exit		= dw_mipi_dsi_dphy_exit,
> +};
> +
>   static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev)
>   {
>   	struct device *dev = &pdev->dev;
>   	struct device_node *np = dev->of_node;
>   	struct dw_mipi_dsi_rockchip *dsi;
> +	struct phy_provider *phy_provider;
>   	struct resource *res;
>   	const struct rockchip_dw_dsi_chip_data *cdata =
>   				of_device_get_match_data(dev);
> @@ -1109,6 +1364,19 @@ static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev)
>   	dsi->pdata.priv_data = dsi;
>   	platform_set_drvdata(pdev, dsi);
>   
> +	mutex_init(&dsi->usage_mutex);
> +
> +	dsi->dphy = devm_phy_create(dev, NULL, &dw_mipi_dsi_dphy_ops);
> +	if (IS_ERR(dsi->dphy)) {
> +		DRM_DEV_ERROR(&pdev->dev, "failed to create PHY\n");
> +		return PTR_ERR(dsi->dphy);
> +	}
> +
> +	phy_set_drvdata(dsi->dphy, dsi);
> +	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +	if (IS_ERR(phy_provider))
> +		return PTR_ERR(phy_provider);
> +
>   	dsi->dmd = dw_mipi_dsi_probe(pdev, &dsi->pdata);
>   	if (IS_ERR(dsi->dmd)) {
>   		ret = PTR_ERR(dsi->dmd);
> @@ -1175,6 +1443,75 @@ static const struct rockchip_dw_dsi_chip_data rk3288_chip_data[] = {
>   	{ /* sentinel */ }
>   };
>   
> +static int rk3399_dphy_tx1rx1_init(struct phy *phy)
> +{
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +
> +	/*
> +	 * Set TX1RX1 source to isp1.
> +	 * Assume ISP0 is supplied by the RX0 dphy.
> +	 */
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON24,
> +		     HIWORD_UPDATE(0, RK3399_TXRX_SRC_SEL_ISP0));
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON24,
> +		     HIWORD_UPDATE(0, RK3399_TXRX_MASTERSLAVEZ));
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON24,
> +		     HIWORD_UPDATE(0, RK3399_TXRX_BASEDIR));
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON23,
> +		     HIWORD_UPDATE(0, RK3399_DSI1_ENABLE));
> +
> +	return 0;
> +}
> +
> +static int rk3399_dphy_tx1rx1_power_on(struct phy *phy)
> +{
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +
> +	/* tester reset pulse */
> +	dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLK | PHY_TESTCLR);
> +	usleep_range(100, 150);
> +
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON24,
> +		     HIWORD_UPDATE(0, RK3399_TXRX_MASTERSLAVEZ));
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON24,
> +		     HIWORD_UPDATE(RK3399_TXRX_BASEDIR, RK3399_TXRX_BASEDIR));
> +
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON23,
> +		     HIWORD_UPDATE(0, RK3399_DSI1_FORCERXMODE));
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON23,
> +		     HIWORD_UPDATE(0, RK3399_DSI1_FORCETXSTOPMODE));
> +
> +	/* Disable lane turn around, which is ignored in receive mode */
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON24,
> +		     HIWORD_UPDATE(0, RK3399_TXRX_TURNREQUEST));
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON23,
> +		     HIWORD_UPDATE(RK3399_DSI1_TURNDISABLE,
> +				   RK3399_DSI1_TURNDISABLE));
> +	usleep_range(100, 150);
> +
> +	dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLK | PHY_UNTESTCLR);
> +	usleep_range(100, 150);
> +
> +	/* Enable dphy lanes */
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON23,
> +		     HIWORD_UPDATE(GENMASK(dsi->dphy_config.lanes - 1, 0),
> +				   RK3399_DSI1_ENABLE));
> +
> +	usleep_range(100, 150);
> +
> +	return 0;
> +}
> +
> +static int rk3399_dphy_tx1rx1_power_off(struct phy *phy)
> +{
> +	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +
> +	regmap_write(dsi->grf_regmap, RK3399_GRF_SOC_CON23,
> +		     HIWORD_UPDATE(0, RK3399_DSI1_ENABLE));
> +
> +	return 0;
> +}
> +
>   static const struct rockchip_dw_dsi_chip_data rk3399_chip_data[] = {
>   	{
>   		.reg = 0xff960000,
> @@ -1217,6 +1554,10 @@ static const struct rockchip_dw_dsi_chip_data rk3399_chip_data[] = {
>   
>   		.flags = DW_MIPI_NEEDS_PHY_CFG_CLK | DW_MIPI_NEEDS_GRF_CLK,
>   		.max_data_lanes = 4,
> +
> +		.dphy_rx_init = rk3399_dphy_tx1rx1_init,
> +		.dphy_rx_power_on = rk3399_dphy_tx1rx1_power_on,
> +		.dphy_rx_power_off = rk3399_dphy_tx1rx1_power_off,
>   	},
>   	{ /* sentinel */ }
>   };
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel