Re: [PATCH] drm/rockchip: vop2: Drop unused if_dclk_rate variable

[Cristian Ciocaltea]

Hi Andy,

On 1/5/24 11:13, Andy Yan wrote:
> Hi Cristian:
> 
> On 1/5/24 03:12, Cristian Ciocaltea wrote:
>> Hi Heiko,
>>
>> On 1/4/24 17:58, Heiko Stübner wrote:
>>> Hi Christian, Andy,
>>>
>>> Am Donnerstag, 4. Januar 2024, 15:39:50 CET schrieb Cristian Ciocaltea:
>>>> Commit 5a028e8f062f ("drm/rockchip: vop2: Add support for rk3588")
>>>> introduced a variable which ended up being unused.  Remove it.
>>>>
>>>> rockchip_drm_vop2.c:1688:23: warning: variable ‘if_dclk_rate’ set
>>>> but not used [-Wunused-but-set-variable]
>>>>
>>>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@xxxxxxxxxxxxx>

[...]

>> The current implementation is not able to handle all display modes
>> supported by connected displays, e.g. in my testing environment I
>> encountered issues with 2560x1440-75.00Hz, 2048x1152-60.00Hz,
>> 1024x768-60.00Hz.  Additionally, it doesn't seem to cope well with
>> non-integer refresh rates like 59.94, 29.97, 23.98, etc.
> 
> I think this is because the thee PLL from cru can't divide accurate
> clock for these
> non-integer refresh rates.
>>
>> My temporary workaround relies on using the HDMI PHY PLL in conjunction
>> with a downstream-based hack to compute the clock rates.  I'm not sure
>> that would be an upstreamable solution, so I would let Andy shed some
>> light on the topic.
> 
> Yes, use PLL from HDMI PHY can give more flexible clock rates to support
> more display mode.
> We also use it in our bsp kernel, but one thing should  keep in mind
> that  use HDMI PHY pll
> as dclk source can only work for HDMI 2.0 or bellow, if can't be used in
> hdmi 2.1 mode(such as 4K120),
> so we need to switch the clock source by hdmi work mode.
> 
> The difficult thing is how to make this accepted by upstream.

Thanks for the heads-up!  I will try to have a look as soon as I
finalize the work on HDMI phy & bridge drivers (unfortunately I had to
put it temporarily on hold to focus on different tasks, but I expect to
resume soon).

Regards,
Cristian