Received-SPF: pass (google.com: domain of linux-kernel+bounces-70364-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <4c66c084-5af6-43fb-a256-4746b2e00192@rock-chips.com>
Date: Sun, 18 Feb 2024 19:14:56 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] drm: rockchip: Don't require MIPI DSI device when it's
 used for ISP
Content-Language: en-US
To: =?UTF-8?Q?Ond=C5=99ej_Jirman?= <megi@xff.cz>, linux-kernel@vger.kernel.org
Cc: Sandy Huang <hjc@rock-chips.com>, =?UTF-8?Q?Heiko_St=C3=BCbner?=
 <heiko@sntech.de>, Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
 Maxime Ripard <mripard@kernel.org>, Thomas Zimmermann <tzimmermann@suse.de>,
 David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>,
 "open list:DRM DRIVERS FOR ROCKCHIP" <dri-devel@lists.freedesktop.org>,
 "moderated list:ARM/Rockchip SoC support"
 <linux-arm-kernel@lists.infradead.org>,
 "open list:ARM/Rockchip SoC support" <linux-rockchip@lists.infradead.org>
References: <20240217183941.1752463-1-megi@xff.cz>
From: Andy Yan <andy.yan@rock-chips.com>
In-Reply-To: <20240217183941.1752463-1-megi@xff.cz>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

Hi,

On 2/18/24 02:39, Ondřej Jirman wrote:
> From: Ondrej Jirman <megi@xff.cz>
> 
> On RK3399 one MIPI DSI device can be alternatively used with the ISP1,
> to provide RX DPHY. When this is the case (ISP1 is enabled in device
> tree), probe success of DRM is tied to probe success of ISP1 connected
> camera sensor. This can fail if the user is able to killswitch the camera
> power, like on Pinephone Pro.
> 
> Detect use of MIPI DSI controller by ISP, and don't include it in
> component match list in that case.
> 

Isn't this supposed to be taken care of within the dts?
Since DPHY1 should exclusively used by MIPI DSI1 and ISP1, then if
a device want to use ISP1, the DSI1 should be disabled in dts.
> Signed-off-by: Ondrej Jirman <megi@xff.cz>
> ---
>   drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 47 +++++++++++++++++++++
>   1 file changed, 47 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> index ab55d7132550..f47de94ad576 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> @@ -354,6 +354,43 @@ static void rockchip_drm_match_remove(struct device *dev)
>   		device_link_del(link);
>   }
>   
> +/*
> + * Check if ISP block linked to a mipi-dsi device via phys phandle is
> + * enabled in device tree.
> + */
> +static bool rockchip_drm_is_mipi1_and_used_by_isp(struct device *dev)
> +{
> +	struct device_node *np = NULL, *phy_np;
> +
> +	if (!of_device_is_compatible(dev->of_node, "rockchip,rk3399-mipi-dsi"))
> +		return false;
> +
> +	while (true) {
> +		np = of_find_compatible_node(np, NULL, "rockchip,rk3399-cif-isp");
> +		if (!np)
> +			break;
> +
> +		if (!of_device_is_available(np)) {
> +			of_node_put(np);
> +			continue;
> +		}
> +
> +		phy_np = of_parse_phandle(np, "phys", 0);
> +		if (!phy_np) {
> +			of_node_put(np);
> +			continue;
> +		}
> +
> +		of_node_put(phy_np);
> +		of_node_put(np);
> +
> +		if (phy_np == dev->of_node)
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
>   static struct component_match *rockchip_drm_match_add(struct device *dev)
>   {
>   	struct component_match *match = NULL;
> @@ -371,6 +408,16 @@ static struct component_match *rockchip_drm_match_add(struct device *dev)
>   			if (!d)
>   				break;
>   
> +			/*
> +			 * If mipi1 is connected to ISP, we don't want to wait for mipi1 component,
> +			 * because it will not be used by DRM anyway, to not tie success of camera
> +			 * driver probe to display pipeline initialization.
> +			 */
> +			if (rockchip_drm_is_mipi1_and_used_by_isp(d)) {
> +				dev_info(d, "used by ISP1, skipping from DRM\n");
> +				continue;
> +			}
> +
>   			device_link_add(dev, d, DL_FLAG_STATELESS);
>   			component_match_add(dev, &match, component_compare_dev, d);
>   		} while (true);