lkml.org 
[lkml]   [2024]   [Feb]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] drm: rockchip: Don't require MIPI DSI device when it's used for ISP
From
Hi Ondrej:

On 2/18/24 23:17, Ondřej Jirman wrote:
> Hi Andy,
>
> On Sun, Feb 18, 2024 at 07:14:56PM +0800, Andy Yan wrote:
>> 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.
>
> DSI1 can't be disabled, because it provides PHY device for ISP1 in this
> scenario.
>
> The problem is that in this scenario DRM keeps waiting for DSI1 device,
> despite it just being used for PHY for ISP1 and not as a component for
> rockchip DRM driver.


Oh, get it.
With a quick look, seems that Heiko has already take this scenario into consideration
when add PHY function in the dsi drivers[0]
Does this mean the current drivers does not work as expected?

[0]https://patchwork.freedesktop.org/patch/420386/
>
> See:
>
> isp1: isp1@ff920000 {
> compatible = "rockchip,rk3399-cif-isp";
> reg = <0x0 0xff920000 0x0 0x4000>;
> interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH 0>;
> clocks = <&cru SCLK_ISP1>,
> <&cru ACLK_ISP1_WRAPPER>,
> <&cru HCLK_ISP1_WRAPPER>;
> clock-names = "isp", "aclk", "hclk";
> iommus = <&isp1_mmu>;
> phys = <&mipi_dsi1>; <---------
> phy-names = "dphy";
> power-domains = <&power RK3399_PD_ISP1>;
> status = "disabled";
>
> If mipi_dsi1 is disabled, isp1 will never probe.
>
> It's a consequence of this special dual use of mipi_dsi1.
>
> kind regards,
> o.
>
>>> 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);

\
 
 \ /
  Last update: 2024-05-27 15:09    [W:0.262 / U:0.028 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site