Re: [RFC PATCH] drm/msm: lookup the ICC paths in both mdp5/dpu and mdss devices

From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
To: Marijn Suijten <marijn.suijten@somainline.org>
Cc: freedreno@lists.freedesktop.org,
	Yassine Oudjana <y.oudjana@protonmail.com>,
	David Airlie <airlied@linux.ie>,
	linux-arm-msm@vger.kernel.org,
	Abhinav Kumar <quic_abhinavk@quicinc.com>,
	dri-devel@lists.freedesktop.org,
	Stephen Boyd <swboyd@chromium.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Sean Paul <sean@poorly.run>
Subject: Re: [RFC PATCH] drm/msm: lookup the ICC paths in both mdp5/dpu and mdss devices
Date: Wed, 19 Oct 2022 12:27:21 +0300	[thread overview]
Message-ID: <09e82b02-dffd-8ec9-4851-11715a16b67b@linaro.org> (raw)
In-Reply-To: <20221019091341.bqkro3zmnojcqvnu@SoMainline.org>

On 19/10/2022 12:13, Marijn Suijten wrote:
> On 2022-08-26 12:16:40, Dmitry Baryshkov wrote:
>> On 05/08/2022 15:24, Marijn Suijten wrote:
>>> On 2022-08-05 14:56:30, Dmitry Baryshkov wrote:
>>>> The commit 6874f48bb8b0 ("drm/msm: make mdp5/dpu devices master
>>>> components") changed the MDP5 driver to look for the interconnect paths
>>>> in the MDSS device rather than in the MDP5 device itself. This was left
>>>> unnoticed since on my testing devices the interconnects probably didn't
>>>> reach the sync state.
>>>>
>>>> Rather than just using the MDP5 device for ICC path lookups for the MDP5
>>>> devices, introduce an additional helper to check both MDP5/DPU and MDSS
>>>> nodes. This will be helpful for the MDP5->DPU conversion, since the
>>>> driver will have to check both nodes.
>>>>
>>>> Fixes: 6874f48bb8b0 ("drm/msm: make mdp5/dpu devices master components")
>>>> Reported-by: Marijn Suijten <marijn.suijten@somainline.org>
>>>> Reported-by: Yassine Oudjana <y.oudjana@protonmail.com>
>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>
>>> Tested-by: Marijn Suijten <marijn.suijten@somainline.org> # On sdm630
>>>
>>> But I'm not sure about giving my Reviewed-by to this, as I'd rather
>>> *correct* the DT bindings for sdm630 and msm8996 to provide
>>> interconnects in the MDSS node unless there are strong reasons not to
>>> (and I don't consider "backwards compatibility" to be one, this binding
>>> "never even existed" if mdp5.txt is to be believed).
>>
>> As a kind of a joke, I'd prefer to have interconnects in the mdp/dpu
>> device node. In the end, the interconnects describe the path between the
>> display controller and the DDR, not the path between the whole MDSS and DDR.
>>
>> So, for next chipsets I'd vote to move icc to dpu/mdp node (and maybe
>> even move existing inerconnects to the dpu node).
> 
> Sure.  In that case, do you want to rework this patch / code again to
> only look in the DPU/MDP, and not at MDSS at all?  (Or is that another
> DT API break we'd rather not make?)

I'd rather not make this break. Let's keep backwards compatibility at 
least for now.

-- 
With best wishes
Dmitry