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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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>

in general, please don't send non-series patches as replies to other patches.
It confuses tooling like b4 way too often, as this patch is not designated
as a 2/2 (similar to the first one not being 1/2).

That was unintentional, sorry!  I wrongly assumed 'git send-email' is
able to correctly handle multiple patches which are not part of a
series.  I'm not sure if the '--no-thread' flag would have helped.

---
  drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 2 --
  1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index 44508c2dd614..923985d4161b 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -1685,7 +1685,6 @@ static unsigned long rk3588_calc_cru_cfg(struct vop2_video_port *vp, int id,
  	unsigned long dclk_core_rate = v_pixclk >> 2;
  	unsigned long dclk_rate = v_pixclk;
  	unsigned long dclk_out_rate;
-	unsigned long if_dclk_rate;
  	unsigned long if_pixclk_rate;
  	int K = 1;
@@ -1700,7 +1699,6 @@ static unsigned long rk3588_calc_cru_cfg(struct vop2_video_port *vp, int id,
  		}
if_pixclk_rate = (dclk_core_rate << 1) / K;
-		if_dclk_rate = dclk_core_rate / K;
  		/*
  		 * *if_pixclk_div = dclk_rate / if_pixclk_rate;
  		 * *if_dclk_div = dclk_rate / if_dclk_rate;
  		 */
		*if_pixclk_div = 2;
		*if_dclk_div = 4;

with the code continuing with those static constants but the comment
showing a forumula, I do hope Andy can provide a bit of insight into
what is happening here.

I.e. I'd really like to understand if that really is just a remnant or
something different is needed.

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,
Cristian


Heiko



_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-rockchip



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux