| Message ID | d67bf9c42c479f8ee60e32a097ce63d227eca912.1497006245.git.joabreu@synopsys.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@synopsys.com> >>> Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> >>> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> >>> Cc: Archit Taneja <architt@codeaurora.org> >>> Cc: Andrzej Hajda <a.hajda@samsung.com> >>> Cc: Mark Yao <mark.yao@rock-chips.com> >>> Cc: Carlos Palminha <palminha@synopsys.com> >>> --- >>> >>> 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",
Hi Jose, Am Freitag, 9. Juni 2017, 13:53:12 CEST schrieb Jose Abreu: > 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). while I'm not Mark and haven't got a working display chain in my test setup yet, I can at least provide the requested phy-id. The value in CONFIG2_ID is 0xf3 on the rk3399 (same on rk3368), which of course resolves to DW_HDMI_PHY_DWC_HDMI20_TX_PHY. Heiko > > 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@synopsys.com> > > Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > Cc: Archit Taneja <architt@codeaurora.org> > > Cc: Andrzej Hajda <a.hajda@samsung.com> > > Cc: Mark Yao <mark.yao@rock-chips.com> > > Cc: Carlos Palminha <palminha@synopsys.com> > > --- > > > > 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", > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
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. So: Tested-by: Mark Yao <mark.yao@rock-chips.com> 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@synopsys.com> >>>> Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> >>>> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> >>>> Cc: Archit Taneja <architt@codeaurora.org> >>>> Cc: Andrzej Hajda <a.hajda@samsung.com> >>>> Cc: Mark Yao <mark.yao@rock-chips.com> >>>> Cc: Carlos Palminha <palminha@synopsys.com> >>>> --- >>>> >>>> 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", > > >
Hi, Jose：
The value of RK3399 reg HDMI_CONFIG2_ID is 0xf3.
Using hdmi_phy_configure_dwc_hdmi_3d_tx() for hdmi 2.0 phy, we test
all HDMI video mode(including 594MHz) and it woks good.
And our customer has pass the HDMI CTS.
If you send a patch with a general config function for HDMI 2.0
phys, we can test it on our platform.
福州瑞芯微电子股份有限公司 郑阳
Email: zhengyang@rock-chips.com
在 2017年06月22日 16:25, Mark yao 写道:
> 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.
>
> So:
> Tested-by: Mark Yao <mark.yao@rock-chips.com>
>
> 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@synopsys.com>
>>>>> Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>>>>> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>>>>> Cc: Archit Taneja <architt@codeaurora.org>
>>>>> Cc: Andrzej Hajda <a.hajda@samsung.com>
>>>>> Cc: Mark Yao <mark.yao@rock-chips.com>
>>>>> Cc: Carlos Palminha <palminha@synopsys.com>
>>>>> ---
>>>>>
>>>>> 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",
>>
>>
>>
>
>
Hi Jose, On Tuesday 13 Jun 2017 15:11:27 Jose Abreu wrote: > Hi Laurent, > > Sorry for the late reply! No worries. > On 10-06-2017 09:50, Laurent Pinchart wrote: > > 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? The highest resolution I've personally tested is 1440x900. I'd need to buy another monitor to go higher than that, but maybe this is the excuse I've been waiting for :-) > If I send a patch with a general config function for HDMI 2.0 phys can you > test it on your platform? Sure, I'd be glad to.
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",
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. 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@synopsys.com> Cc: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> Cc: Archit Taneja <architt@codeaurora.org> Cc: Andrzej Hajda <a.hajda@samsung.com> Cc: Mark Yao <mark.yao@rock-chips.com> Cc: Carlos Palminha <palminha@synopsys.com> --- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 1 + 1 file changed, 1 insertion(+)