Re: [PATCH] drm: bridge: synopsys/dw-hdmi: Provide default configuration function for HDMI 2.0 PHY

From: Mark yao
Date: Thu Jun 22 2017 - 04:25:38 EST

Hi Jose

Sorry miss your email and Sorry for the late reply

I can sure that your patch works on our rk3399 platform.

my internal kernel already has similar patch, using hdmi_phy_configure_dwc_hdmi_3d_tx() for hdmi 2.0 phy,
good works with many video modes (4k, 1080p, 720p etc.), I'm not familiar with hdmi, but hdmi display actually works for us.

And, I had tried the 4.12-rc1 kernel with your patch, display works good.

Tested-by: Mark Yao <mark.yao@xxxxxxxxxxxxxx>

On 2017å06æ13æ 22:11, Jose Abreu wrote:
Hi Laurent,

Sorry for the late reply!

On 10-06-2017 09:50, Laurent Pinchart wrote:
Hi Jose,

On Friday 09 Jun 2017 13:53:12 Jose Abreu wrote:
On 09-06-2017 12:04, Jose Abreu wrote:
Currently HDMI 2.0 PHYs do not have a default configuration function.

As these PHYs have the same register layout as the 3D PHYs we can
safely use the default configuration function.
I may have been a little to fast arriving at this conclusion. I
mean most of the registers match but in the configuration
function there are registers that do not match. Did you actually
test this configuration function with an HDMI 2.0 phy? And did
you test with different video modes? From my experience the phy
may be wrongly configured and sometimes work anyway.

Do please retest with as many video modes as you can and give me
your phy ID (read from controller config reg HDMI_CONFIG2_ID).
The Renesas R-Car Gen3 HDMI PHY reports an DWC HDMI 2.0 TX PHY ID, but has a
configuration function (rcar_hdmi_phy_configure() in drivers/gpu/drm/rcar-
du/rcar_dw_hdmi.c) that doesn't match hdmi_phy_configure_dwc_hdmi_3d_tx().
From the information I have been given the layout of the configuration
registers haven't been changed by Renesas. I know we've briefly discussed this
in the past, but I'd appreciate if you could have a second look and tell me
what you think.
Yup, yours seems correct. Though at the time you submitted I
found it odd that only 3 registers needed to be written whilst
for HDMI 2.0 phys I have here 6 registers, but you said it is
working so I though your phy was different ...

Even so, one thing I would like to know is what was the max
resolution you tested? I see you have clock values up to 297MHz,
so 4k@30Hz? If I send a patch with a general config function for
HDMI 2.0 phys can you test it on your platform?

Best regards,
Jose Miguel Abreu

If, for some reason,

the PHY is custom this change will not make any impact because
in configuration function we prefer the pdata provided configuration
function over the internal one.

This patch is based on today's drm-misc-next branch.

Signed-off-by: Jose Abreu <joabreu@xxxxxxxxxxxx>
Cc: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx>
Cc: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
Cc: Archit Taneja <architt@xxxxxxxxxxxxxx>
Cc: Andrzej Hajda <a.hajda@xxxxxxxxxxx>
Cc: Mark Yao <mark.yao@xxxxxxxxxxxxxx>
Cc: Carlos Palminha <palminha@xxxxxxxxxxxx>

drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index ead1124..10c8d8c 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -2170,6 +2170,7 @@ static irqreturn_t dw_hdmi_irq(int irq, void
.name = "DWC HDMI 2.0 TX PHY",
.gen = 2,
.has_svsret = true,
+ .configure = hdmi_phy_configure_dwc_hdmi_3d_tx,
}, {
.name = "Vendor PHY",

ïark Yao