From: Quanyang Wang <[email protected]>
The Runtime PM subsystem will force the device "fd4a0000.zynqmp-display"
to enter suspend state while booting if the following conditions are met:
- the usage counter is zero (pm_runtime_get_sync hasn't been called yet)
- no 'active' children (no zynqmp-dp-snd-xx node under dpsub node)
- no other device in the same power domain (dpdma node has no
"power-domains = <&zynqmp_firmware PD_DP>" property)
So there is a scenario as below:
1) DP device enters suspend state <- call zynqmp_gpd_power_off
2) zynqmp_disp_crtc_setup_clock <- configurate register VPLL_FRAC_CFG
3) pm_runtime_get_sync <- call zynqmp_gpd_power_on and clear previous
VPLL_FRAC_CFG configuration
4) clk_prepare_enable(disp->pclk) <- enable failed since VPLL_FRAC_CFG
configuration is corrupted
From above, we can see that pm_runtime_get_sync may clear register
VPLL_FRAC_CFG configuration and result the failure of clk enabling.
Putting pm_runtime_get_sync at the very beginning of the function
zynqmp_disp_crtc_atomic_enable can resolve this issue.
Signed-off-by: Quanyang Wang <[email protected]>
---
drivers/gpu/drm/xlnx/zynqmp_disp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
index 148add0ca1d6..909e6c265406 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
@@ -1445,9 +1445,10 @@ zynqmp_disp_crtc_atomic_enable(struct drm_crtc *crtc,
struct drm_display_mode *adjusted_mode = &crtc->state->adjusted_mode;
int ret, vrefresh;
+ pm_runtime_get_sync(disp->dev);
+
zynqmp_disp_crtc_setup_clock(crtc, adjusted_mode);
- pm_runtime_get_sync(disp->dev);
ret = clk_prepare_enable(disp->pclk);
if (ret) {
dev_err(disp->dev, "failed to enable a pixel clock\n");
--
2.25.1
Ping.
Any comment on this patch?
Thanks,
Quanyang
On 3/10/21 12:59 PM, [email protected] wrote:
> From: Quanyang Wang <[email protected]>
>
> The Runtime PM subsystem will force the device "fd4a0000.zynqmp-display"
> to enter suspend state while booting if the following conditions are met:
> - the usage counter is zero (pm_runtime_get_sync hasn't been called yet)
> - no 'active' children (no zynqmp-dp-snd-xx node under dpsub node)
> - no other device in the same power domain (dpdma node has no
> "power-domains = <&zynqmp_firmware PD_DP>" property)
>
> So there is a scenario as below:
> 1) DP device enters suspend state <- call zynqmp_gpd_power_off
> 2) zynqmp_disp_crtc_setup_clock <- configurate register VPLL_FRAC_CFG
> 3) pm_runtime_get_sync <- call zynqmp_gpd_power_on and clear previous
> VPLL_FRAC_CFG configuration
> 4) clk_prepare_enable(disp->pclk) <- enable failed since VPLL_FRAC_CFG
> configuration is corrupted
>
> From above, we can see that pm_runtime_get_sync may clear register
> VPLL_FRAC_CFG configuration and result the failure of clk enabling.
> Putting pm_runtime_get_sync at the very beginning of the function
> zynqmp_disp_crtc_atomic_enable can resolve this issue.
>
> Signed-off-by: Quanyang Wang <[email protected]>
> ---
> drivers/gpu/drm/xlnx/zynqmp_disp.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> index 148add0ca1d6..909e6c265406 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> @@ -1445,9 +1445,10 @@ zynqmp_disp_crtc_atomic_enable(struct drm_crtc *crtc,
> struct drm_display_mode *adjusted_mode = &crtc->state->adjusted_mode;
> int ret, vrefresh;
>
> + pm_runtime_get_sync(disp->dev);
> +
> zynqmp_disp_crtc_setup_clock(crtc, adjusted_mode);
>
> - pm_runtime_get_sync(disp->dev);
> ret = clk_prepare_enable(disp->pclk);
> if (ret) {
> dev_err(disp->dev, "failed to enable a pixel clock\n");
Hi Quanyang,
Thank you for the patch.
On Wed, Mar 10, 2021 at 12:59:45PM +0800, [email protected] wrote:
> From: Quanyang Wang <[email protected]>
>
> The Runtime PM subsystem will force the device "fd4a0000.zynqmp-display"
> to enter suspend state while booting if the following conditions are met:
> - the usage counter is zero (pm_runtime_get_sync hasn't been called yet)
> - no 'active' children (no zynqmp-dp-snd-xx node under dpsub node)
> - no other device in the same power domain (dpdma node has no
> "power-domains = <&zynqmp_firmware PD_DP>" property)
>
> So there is a scenario as below:
> 1) DP device enters suspend state <- call zynqmp_gpd_power_off
> 2) zynqmp_disp_crtc_setup_clock <- configurate register VPLL_FRAC_CFG
> 3) pm_runtime_get_sync <- call zynqmp_gpd_power_on and clear previous
> VPLL_FRAC_CFG configuration
> 4) clk_prepare_enable(disp->pclk) <- enable failed since VPLL_FRAC_CFG
> configuration is corrupted
>
> From above, we can see that pm_runtime_get_sync may clear register
> VPLL_FRAC_CFG configuration and result the failure of clk enabling.
> Putting pm_runtime_get_sync at the very beginning of the function
> zynqmp_disp_crtc_atomic_enable can resolve this issue.
Isn't this an issue in the firmware though, which shouldn't clear the
previous VPLLF_FRAC_CFG ?
> Signed-off-by: Quanyang Wang <[email protected]>
Nonetheless, this change looks good to me, I actually had the same patch
in my tree while investigation issues related to the clock rate, so
Reviewed-by: Laurent Pinchart <[email protected]>
Tested-by: Laurent Pinchart <[email protected]>
I was hoping it would solve the issue I'm experiencing with the DP
clock, but that's not the case :-( In a nutshell, when the DP is first
started, the clock frequency is incorrect. The following quick & dirty
patch fixes the problem:
diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
index 74ac0a064eb5..fdbe1b0640aa 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
@@ -1439,6 +1439,10 @@ zynqmp_disp_crtc_atomic_enable(struct drm_crtc *crtc,
pm_runtime_get_sync(disp->dev);
+ ret = clk_prepare_enable(disp->pclk);
+ if (!ret)
+ clk_disable_unprepare(disp->pclk);
+
zynqmp_disp_crtc_setup_clock(crtc, adjusted_mode);
ret = clk_prepare_enable(disp->pclk);
The problem doesn't seem to be in the kernel, but on the TF-A or PMU
firmware side. Have you experienced this by any chance ?
> ---
> drivers/gpu/drm/xlnx/zynqmp_disp.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> index 148add0ca1d6..909e6c265406 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> @@ -1445,9 +1445,10 @@ zynqmp_disp_crtc_atomic_enable(struct drm_crtc *crtc,
> struct drm_display_mode *adjusted_mode = &crtc->state->adjusted_mode;
> int ret, vrefresh;
>
> + pm_runtime_get_sync(disp->dev);
> +
> zynqmp_disp_crtc_setup_clock(crtc, adjusted_mode);
>
> - pm_runtime_get_sync(disp->dev);
> ret = clk_prepare_enable(disp->pclk);
> if (ret) {
> dev_err(disp->dev, "failed to enable a pixel clock\n");
--
Regards,
Laurent Pinchart
Hi Laurent,
On 3/17/21 4:32 AM, Laurent Pinchart wrote:
> Hi Quanyang,
>
> Thank you for the patch.
>
> On Wed, Mar 10, 2021 at 12:59:45PM +0800, [email protected] wrote:
>> From: Quanyang Wang <[email protected]>
>>
>> The Runtime PM subsystem will force the device "fd4a0000.zynqmp-display"
>> to enter suspend state while booting if the following conditions are met:
>> - the usage counter is zero (pm_runtime_get_sync hasn't been called yet)
>> - no 'active' children (no zynqmp-dp-snd-xx node under dpsub node)
>> - no other device in the same power domain (dpdma node has no
>> "power-domains = <&zynqmp_firmware PD_DP>" property)
>>
>> So there is a scenario as below:
>> 1) DP device enters suspend state <- call zynqmp_gpd_power_off
>> 2) zynqmp_disp_crtc_setup_clock <- configurate register VPLL_FRAC_CFG
>> 3) pm_runtime_get_sync <- call zynqmp_gpd_power_on and clear previous
>> VPLL_FRAC_CFG configuration
>> 4) clk_prepare_enable(disp->pclk) <- enable failed since VPLL_FRAC_CFG
>> configuration is corrupted
>>
>> From above, we can see that pm_runtime_get_sync may clear register
>> VPLL_FRAC_CFG configuration and result the failure of clk enabling.
>> Putting pm_runtime_get_sync at the very beginning of the function
>> zynqmp_disp_crtc_atomic_enable can resolve this issue.
> Isn't this an issue in the firmware though, which shouldn't clear the
> previous VPLLF_FRAC_CFG ?
Thank you for your review. I used to look into the atf and PWU code and
it seems (I didn't add debug code
to PMU to make sure if PMU really does this, I only in kernel call
zynqmp_pm_get_pll_frac_data to make sure that
the value in data field of VPLL_FRAC_CFG register is changed from 0x4000
to 0x0 after run pm_runtime_get_sync)
that PMU intends to reset VPLL when there is an Off and On in DP
Powerdomain.
Linux ATF PWU
zynqmp_gpd_power_on
->zynqmp_pm_set_requirement
-->send PM_SET_REQUIREMENT to ATF ==> ATF send ipi to PWU ==> PmSetRequirement
->PmRequirementUpdate
-->PmUpdateSlave(masterReq->slave)
--->PmSlaveChangeState
---->PmSlaveChangeState
----->PmSlaveClearAfterState
------>PmClockRelease
------->PmClockReleaseInt(&ch->clock->base)
-------->clk->class->release(clk)
--------->PmPllBypassAndReset //Here reset the VPLL then VPLL_FRAC_CFG is cleared.
>
>> Signed-off-by: Quanyang Wang <[email protected]>
> Nonetheless, this change looks good to me, I actually had the same patch
> in my tree while investigation issues related to the clock rate, so
>
> Reviewed-by: Laurent Pinchart <[email protected]>
> Tested-by: Laurent Pinchart <[email protected]>
>
> I was hoping it would solve the issue I'm experiencing with the DP
> clock, but that's not the case :-( In a nutshell, when the DP is first
> started, the clock frequency is incorrect. The following quick & dirty
> patch fixes the problem:
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> index 74ac0a064eb5..fdbe1b0640aa 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> @@ -1439,6 +1439,10 @@ zynqmp_disp_crtc_atomic_enable(struct drm_crtc *crtc,
>
> pm_runtime_get_sync(disp->dev);
>
> + ret = clk_prepare_enable(disp->pclk);
> + if (!ret)
> + clk_disable_unprepare(disp->pclk);
> +
> zynqmp_disp_crtc_setup_clock(crtc, adjusted_mode);
>
> ret = clk_prepare_enable(disp->pclk);
>
> The problem doesn't seem to be in the kernel, but on the TF-A or PMU
> firmware side. Have you experienced this by any chance ?
Yes, I bumped into the same issue and I used to make a patch (Patch 1)
as below.
I didn't send it to mainline because it seems not to be a driver issue.
The mode of VPLL
is not set correctly because:
1) VPLL is enabled before linux
2) linux calling pm_clock_set_pll_mode can't really set register because
in ATF
it only store the mode value to a structure and wait a clk-enable
request to do
the register-set operation.
3) linux call clk_enable will not send a clk-enable request since it
checks that
the VPLL is already hardware-enabled because of 1).
So the firmware should disable VPLL when it exits and also in linux
zynqmp clk driver
there should be a check list to reset some clks to a predefined state.
By the way, there is a tiny patch (Patch 2) to fix the black screen
issue in DP. I think you may
be preparing a big patch which add drm properties to this driver and it
may contain this modification,
so I didn't send it.
Thanks,
Quanyang
Patch 1:
From 93311de2c61e87f2426b89259d81cded71aee673 Mon Sep 17 00:00:00 2001
From: Quanyang Wang <[email protected]>
Date: Thu, 3 Dec 2020 19:19:50 +0800
Subject: [PATCH 1/3] drm: xlnx: set PLL_MODE_FRAC mode to VPLL_FRAC_CFG by
re-enabling disp->pclk
When the function clk_set_rate configures the rate of disp->pclk,
zynqmp_pm_set_pll_frac_mode will be called to set VPLL's mode to
be PLL_MODE_FRAC or PLL_MODE_INT by invoking an SMC call to ATF.
But in ATF, the service pm_clock_set_pll_mode doesn't really set
the VPLL_FRAC_CFG register but only stores the mode value to
struct pm_pll *pll. The operation that sets the register must be
triggered by zynqmp_pm_clock_enable.
Since disp->pclk is enabled in hardware before linux booting,
clk_prepare_enable will skip over zynqmp_pm_clock_enable. So
we have to enable then disable disp->pclk, and re-enable it to
make sure that zynqmp_pm_clock_enable is triggered and the mode is
set to VPLL_FRAC_CFG. Or else VPLL will work in an incorrect mode.
Signed-off-by: Quanyang Wang <[email protected]>
---
drivers/gpu/drm/xlnx/zynqmp_disp.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c
b/drivers/gpu/drm/xlnx/zynqmp_disp.c
index 98bd48f13fd1..19753ffc424e 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
@@ -1668,6 +1668,11 @@ int zynqmp_disp_probe(struct zynqmp_dpsub *dpsub,
struct drm_device *drm)
dev_err(disp->dev, "failed to init any video clock\n");
return PTR_ERR(disp->pclk);
}
+
+ /* Make sure that disp->pclk is disabled in hardware */
+ ret = clk_prepare_enable(disp->pclk);
+ clk_disable_unprepare(disp->pclk);
+
disp->pclk_from_ps = true;
}
Patch 2:
From 8c6d36bcb4e79e0e5f8e157446cd994b4a2126f0 Mon Sep 17 00:00:00 2001
From: Quanyang Wang <[email protected]>
Date: Thu, 3 Dec 2020 19:32:56 +0800
Subject: [PATCH 2/3] drm: xlnx: configure alpha value to make graphic layer
opaque
Since graphics layer is primary, and video layer is overaly,
we need to configure the V_BLEND_SET_GLOBAL_ALPHA_REG register
to make graphic layer opaque by default, or else graphic layer
will be transparent and invisible.
Signed-off-by: Quanyang Wang <[email protected]>
---
drivers/gpu/drm/xlnx/zynqmp_disp.c | 3 ++-
drivers/gpu/drm/xlnx/zynqmp_disp_regs.h | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c
b/drivers/gpu/drm/xlnx/zynqmp_disp.c
index 19753ffc424e..5c84589e1899 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
@@ -1468,7 +1468,8 @@ zynqmp_disp_crtc_atomic_enable(struct drm_crtc *crtc,
zynqmp_disp_blend_set_output_format(&disp->blend,
ZYNQMP_DPSUB_FORMAT_RGB);
zynqmp_disp_blend_set_bg_color(&disp->blend, 0, 0, 0);
- zynqmp_disp_blend_set_global_alpha(&disp->blend, false, 0);
+ zynqmp_disp_blend_set_global_alpha(&disp->blend, true,
+ ZYNQMP_DISP_V_BLEND_SET_GLOBAL_ALPHA_MAX);
zynqmp_disp_enable(disp);
diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h
b/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h
index f92a006d5070..ef409aca11ad 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h
+++ b/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h
@@ -22,6 +22,7 @@
#define ZYNQMP_DISP_V_BLEND_SET_GLOBAL_ALPHA 0xc
#define ZYNQMP_DISP_V_BLEND_SET_GLOBAL_ALPHA_VALUE(n) ((n) << 1)
#define ZYNQMP_DISP_V_BLEND_SET_GLOBAL_ALPHA_EN BIT(0)
+#define ZYNQMP_DISP_V_BLEND_SET_GLOBAL_ALPHA_MAX 0xff
#define ZYNQMP_DISP_V_BLEND_OUTPUT_VID_FMT 0x14
#define ZYNQMP_DISP_V_BLEND_OUTPUT_VID_FMT_RGB 0x0
#define ZYNQMP_DISP_V_BLEND_OUTPUT_VID_FMT_YCBCR444 0x1
>> ---
>> drivers/gpu/drm/xlnx/zynqmp_disp.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
>> index 148add0ca1d6..909e6c265406 100644
>> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
>> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
>> @@ -1445,9 +1445,10 @@ zynqmp_disp_crtc_atomic_enable(struct drm_crtc *crtc,
>> struct drm_display_mode *adjusted_mode = &crtc->state->adjusted_mode;
>> int ret, vrefresh;
>>
>> + pm_runtime_get_sync(disp->dev);
>> +
>> zynqmp_disp_crtc_setup_clock(crtc, adjusted_mode);
>>
>> - pm_runtime_get_sync(disp->dev);
>> ret = clk_prepare_enable(disp->pclk);
>> if (ret) {
>> dev_err(disp->dev, "failed to enable a pixel clock\n");
Hi Rohit,
On 3/17/21 7:17 PM, Rohit Visavalia wrote:
> Hi Quanyang & Laurent,
>
> I tested this patch(which moves pm_runtime_get_sync at the very beginning of the function zynqmp_disp_crtc_atomic_enable), i don't see any behavior change with patch, means with patch also DP display is not getting up and it is required to resolution change to make it up.
> Also valid bit is not getting set for VPLL_FRAC_CFG on bootup, same behavior as without patch.
Thank you for your test.
This patch works under the condition of applying the below patch (drm:
xlnx: set PLL_MODE_FRAC mode to VPLL_FRAC_CFG by re-enabling disp->pclk)
(Patch 1) first.
Without Patch 1, the VPLL mode isn't set correctly and
clk_prepare_enable don't take effect. DP monitor can't receive
any signal since the clk is wrong.
After applying Patch 1, there will be error log in the boot message:
[ 1.325602] zynqmp_pll_enable() clock enable failed for vpll_int, ret
= -22
[ 1.325614] zynqmp-dpsub fd4a0000.display: failed to enable a pixel clock
[ 1.456106] ------------[ cut here ]------------
[ 1.456109] [CRTC:33:crtc-0] vblank wait timed out
[ 1.456152] WARNING: CPU: 2 PID: 75 at
drivers/gpu/drm/drm_atomic_helper.c:1512
drm_atomic_helper_wait_for_vblanks.part.0+0x27c/0x298
[ 1.456172] Modules linked in:
[ 1.456181] CPU: 2 PID: 75 Comm: kworker/2:1 Not tainted
5.12.0-rc2-00338-gf78d76e72a46-dirty #4
[ 1.456188] Hardware name: ZynqMP ZCU102 Rev1.0 (DT)
[ 1.456194] Workqueue: events deferred_probe_work_func
Then applying this patch ( drm: xlnx: call pm_runtime_get_sync before
setting pixel clock), this issue will be fixed, DP monitor
will receive signal, but screen is still black. Then applying the below
patch (drm: xlnx: configure alpha value to make graphic layer opaque)
(Patch 2),
the DP will work well.
Thanks,
Quanyang
>
> Thanks,
> Rohit
>
>> -----Original Message-----
>> From: Michal Simek <[email protected]>
>> Sent: Wednesday, March 17, 2021 1:56 PM
>> To: quanyang.wang <[email protected]>; Laurent Pinchart
>> <[email protected]>; Rohit Visavalia
>> <[email protected]>
>> Cc: Hyun Kwon <[email protected]>; David Airlie <[email protected]>;
>> Daniel Vetter <[email protected]>; Michal Simek <[email protected]>; dri-
>> [email protected]; [email protected]; linux-
>> [email protected]
>> Subject: Re: [PATCH] drm: xlnx: call pm_runtime_get_sync before setting pixel
>> clock
>>
>> +Rohit
>>
>> On 3/17/21 4:00 AM, quanyang.wang wrote:
>>> Hi Laurent,
>>>
>>> On 3/17/21 4:32 AM, Laurent Pinchart wrote:
>>>> Hi Quanyang,
>>>>
>>>> Thank you for the patch.
>>>>
>>>> On Wed, Mar 10, 2021 at 12:59:45PM +0800,
>> [email protected]
>>>> wrote:
>>>>> From: Quanyang Wang <[email protected]>
>>>>>
>>>>> The Runtime PM subsystem will force the device "fd4a0000.zynqmp-
>> display"
>>>>> to enter suspend state while booting if the following conditions are
>>>>> met:
>>>>> - the usage counter is zero (pm_runtime_get_sync hasn't been called
>>>>> yet)
>>>>> - no 'active' children (no zynqmp-dp-snd-xx node under dpsub node)
>>>>> - no other device in the same power domain (dpdma node has no
>>>>> "power-domains = <&zynqmp_firmware PD_DP>" property)
>>>>>
>>>>> So there is a scenario as below:
>>>>> 1) DP device enters suspend state <- call zynqmp_gpd_power_off
>>>>> 2) zynqmp_disp_crtc_setup_clock <- configurate register
>>>>> VPLL_FRAC_CFG
>>>>> 3) pm_runtime_get_sync <- call zynqmp_gpd_power_on and
>>>>> clear previous
>>>>> VPLL_FRAC_CFG configuration
>>>>> 4) clk_prepare_enable(disp->pclk) <- enable failed since
>>>>> VPLL_FRAC_CFG
>>>>> configuration is corrupted
>>>>>
>>>>> From above, we can see that pm_runtime_get_sync may clear register
>>>>> VPLL_FRAC_CFG configuration and result the failure of clk enabling.
>>>>> Putting pm_runtime_get_sync at the very beginning of the function
>>>>> zynqmp_disp_crtc_atomic_enable can resolve this issue.
>>>> Isn't this an issue in the firmware though, which shouldn't clear the
>>>> previous VPLLF_FRAC_CFG ?
>>> Thank you for your review. I used to look into the atf and PWU code
>>> and it seems (I didn't add debug code
>>>
>>> to PMU to make sure if PMU really does this, I only in kernel call
>>> zynqmp_pm_get_pll_frac_data to make sure that
>>>
>>> the value in data field of VPLL_FRAC_CFG register is changed from
>>> 0x4000 to 0x0 after run pm_runtime_get_sync)
>>>
>>> that PMU intends to reset VPLL when there is an Off and On in DP
>>> Powerdomain.
>>>
>>>
>>> Linux ATF PWU
>>>
>>> zynqmp_gpd_power_on
>>> ->zynqmp_pm_set_requirement
>>> -->send PM_SET_REQUIREMENT to ATF ==> ATF send ipi to PWU
>>> ==> PmSetRequirement
>>> ->PmRequirementUpdate
>>> -->PmUpdateSlave(masterReq->slave)
>>> --->PmSlaveChangeState
>>> ---->PmSlaveChangeState
>>> ----->PmSlaveClearAfterState
>>> ------>PmClockRelease
>>>
>>> ------->PmClockReleaseInt(&ch->clock->base)
>>> -------->clk->class->release(clk)
>>> --------->PmPllBypassAndReset
>>> //Here reset the VPLL then VPLL_FRAC_CFG is cleared.
>>>
>>>>> Signed-off-by: Quanyang Wang <[email protected]>
>>>> Nonetheless, this change looks good to me, I actually had the same
>>>> patch in my tree while investigation issues related to the clock
>>>> rate, so
>>>>
>>>> Reviewed-by: Laurent Pinchart <[email protected]>
>>>> Tested-by: Laurent Pinchart <[email protected]>
>>>>
>>>> I was hoping it would solve the issue I'm experiencing with the DP
>>>> clock, but that's not the case :-( In a nutshell, when the DP is
>>>> first started, the clock frequency is incorrect. The following quick
>>>> & dirty patch fixes the problem:
>>>>
>>>> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c
>>>> b/drivers/gpu/drm/xlnx/zynqmp_disp.c
>>>> index 74ac0a064eb5..fdbe1b0640aa 100644
>>>> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
>>>> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
>>>> @@ -1439,6 +1439,10 @@ zynqmp_disp_crtc_atomic_enable(struct
>> drm_crtc
>>>> *crtc,
>>>>
>>>> pm_runtime_get_sync(disp->dev);
>>>>
>>>> + ret = clk_prepare_enable(disp->pclk);
>>>> + if (!ret)
>>>> + clk_disable_unprepare(disp->pclk);
>>>> +
>>>> zynqmp_disp_crtc_setup_clock(crtc, adjusted_mode);
>>>>
>>>> ret = clk_prepare_enable(disp->pclk);
>>>>
>>>> The problem doesn't seem to be in the kernel, but on the TF-A or PMU
>>>> firmware side. Have you experienced this by any chance ?
>>> Yes, I bumped into the same issue and I used to make a patch (Patch
>>> 1) as below.
>>>
>>> I didn't send it to mainline because it seems not to be a driver issue.
>>> The mode of VPLL
>>>
>>> is not set correctly because:
>>>
>>> 1) VPLL is enabled before linux
>>>
>>> 2) linux calling pm_clock_set_pll_mode can't really set register
>>> because in ATF
>>>
>>> it only store the mode value to a structure and wait a clk-enable
>>> request to do
>>>
>>> the register-set operation.
>>>
>>> 3) linux call clk_enable will not send a clk-enable request since it
>>> checks that
>>>
>>> the VPLL is already hardware-enabled because of 1).
>>>
>>> So the firmware should disable VPLL when it exits and also in linux
>>> zynqmp clk driver
>>>
>>> there should be a check list to reset some clks to a predefined state.
>>>
>>>
>>> By the way, there is a tiny patch (Patch 2) to fix the black screen
>>> issue in DP. I think you may
>>>
>>> be preparing a big patch which add drm properties to this driver and
>>> it may contain this modification,
>>>
>>> so I didn't send it.
>>>
>>>
>>> Thanks,
>>>
>>> Quanyang
>>>
>>> Patch 1:
>>>
>>> From 93311de2c61e87f2426b89259d81cded71aee673 Mon Sep 17 00:00:00
>> 2001
>>> From: Quanyang Wang <[email protected]>
>>> Date: Thu, 3 Dec 2020 19:19:50 +0800
>>> Subject: [PATCH 1/3] drm: xlnx: set PLL_MODE_FRAC mode to
>>> VPLL_FRAC_CFG by
>>> re-enabling disp->pclk
>>>
>>> When the function clk_set_rate configures the rate of disp->pclk,
>>> zynqmp_pm_set_pll_frac_mode will be called to set VPLL's mode to be
>>> PLL_MODE_FRAC or PLL_MODE_INT by invoking an SMC call to ATF.
>>> But in ATF, the service pm_clock_set_pll_mode doesn't really set the
>>> VPLL_FRAC_CFG register but only stores the mode value to struct pm_pll
>>> *pll. The operation that sets the register must be triggered by
>>> zynqmp_pm_clock_enable.
>>>
>>> Since disp->pclk is enabled in hardware before linux booting,
>>> clk_prepare_enable will skip over zynqmp_pm_clock_enable. So we have
>>> to enable then disable disp->pclk, and re-enable it to make sure that
>>> zynqmp_pm_clock_enable is triggered and the mode is set to
>>> VPLL_FRAC_CFG. Or else VPLL will work in an incorrect mode.
>>>
>>> Signed-off-by: Quanyang Wang <[email protected]>
>>> ---
>>> drivers/gpu/drm/xlnx/zynqmp_disp.c | 5 +++++
>>> 1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c
>>> b/drivers/gpu/drm/xlnx/zynqmp_disp.c
>>> index 98bd48f13fd1..19753ffc424e 100644
>>> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
>>> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
>>> @@ -1668,6 +1668,11 @@ int zynqmp_disp_probe(struct zynqmp_dpsub
>>> *dpsub, struct drm_device *drm)
>>> dev_err(disp->dev, "failed to init any video clock\n");
>>> return PTR_ERR(disp->pclk);
>>> }
>>> +
>>> + /* Make sure that disp->pclk is disabled in hardware */
>>> + ret = clk_prepare_enable(disp->pclk);
>>> + clk_disable_unprepare(disp->pclk);
>>> +
>>> disp->pclk_from_ps = true;
>>> }
>>>
>>> Patch 2:
>>>
>>> From 8c6d36bcb4e79e0e5f8e157446cd994b4a2126f0 Mon Sep 17 00:00:00
>> 2001
>>> From: Quanyang Wang <[email protected]>
>>> Date: Thu, 3 Dec 2020 19:32:56 +0800
>>> Subject: [PATCH 2/3] drm: xlnx: configure alpha value to make graphic
>>> layer
>>> opaque
>>>
>>> Since graphics layer is primary, and video layer is overaly, we need
>>> to configure the V_BLEND_SET_GLOBAL_ALPHA_REG register to make graphic
>>> layer opaque by default, or else graphic layer will be transparent and
>>> invisible.
>>>
>>> Signed-off-by: Quanyang Wang <[email protected]>
>>> ---
>>> drivers/gpu/drm/xlnx/zynqmp_disp.c | 3 ++-
>>> drivers/gpu/drm/xlnx/zynqmp_disp_regs.h | 1 +
>>> 2 files changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c
>>> b/drivers/gpu/drm/xlnx/zynqmp_disp.c
>>> index 19753ffc424e..5c84589e1899 100644
>>> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
>>> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
>>> @@ -1468,7 +1468,8 @@ zynqmp_disp_crtc_atomic_enable(struct drm_crtc
>>> *crtc,
>>> zynqmp_disp_blend_set_output_format(&disp->blend,
>>> ZYNQMP_DPSUB_FORMAT_RGB);
>>> zynqmp_disp_blend_set_bg_color(&disp->blend, 0, 0, 0);
>>> - zynqmp_disp_blend_set_global_alpha(&disp->blend, false, 0);
>>> + zynqmp_disp_blend_set_global_alpha(&disp->blend, true,
>>> + ZYNQMP_DISP_V_BLEND_SET_GLOBAL_ALPHA_MAX);
>>>
>>> zynqmp_disp_enable(disp);
>>>
>>> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h
>>> b/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h
>>> index f92a006d5070..ef409aca11ad 100644
>>> --- a/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h
>>> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp_regs.h
>>> @@ -22,6 +22,7 @@
>>> #define ZYNQMP_DISP_V_BLEND_SET_GLOBAL_ALPHA 0xc
>>> #define ZYNQMP_DISP_V_BLEND_SET_GLOBAL_ALPHA_VALUE(n) ((n) << 1)
>>> #define ZYNQMP_DISP_V_BLEND_SET_GLOBAL_ALPHA_EN BIT(0)
>>> +#define ZYNQMP_DISP_V_BLEND_SET_GLOBAL_ALPHA_MAX 0xff
>>> #define ZYNQMP_DISP_V_BLEND_OUTPUT_VID_FMT 0x14
>>> #define ZYNQMP_DISP_V_BLEND_OUTPUT_VID_FMT_RGB 0x0
>>> #define ZYNQMP_DISP_V_BLEND_OUTPUT_VID_FMT_YCBCR444 0x1
>>>
>>>>> ---
>>>>> drivers/gpu/drm/xlnx/zynqmp_disp.c | 3 ++-
>>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c
>>>>> b/drivers/gpu/drm/xlnx/zynqmp_disp.c
>>>>> index 148add0ca1d6..909e6c265406 100644
>>>>> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
>>>>> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
>>>>> @@ -1445,9 +1445,10 @@ zynqmp_disp_crtc_atomic_enable(struct
>>>>> drm_crtc *crtc,
>>>>> struct drm_display_mode *adjusted_mode =
>>>>> &crtc->state->adjusted_mode;
>>>>> int ret, vrefresh;
>>>>> + pm_runtime_get_sync(disp->dev);
>>>>> +
>>>>> zynqmp_disp_crtc_setup_clock(crtc, adjusted_mode);
>>>>> - pm_runtime_get_sync(disp->dev);
>>>>> ret = clk_prepare_enable(disp->pclk);
>>>>> if (ret) {
>>>>> dev_err(disp->dev, "failed to enable a pixel clock\n");