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

[Jose Abreu]

Hi Mark,


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).

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 *dev_id)
>  		.name = "DWC HDMI 2.0 TX PHY",
>  		.gen = 2,
>  		.has_svsret = true,
> +		.configure = hdmi_phy_configure_dwc_hdmi_3d_tx,
>  	}, {
>  		.type = DW_HDMI_PHY_VENDOR_PHY,
>  		.name = "Vendor PHY",