| Message ID | 185ccf7d4473fa557044732402ca20b3d4007952.1498209896.git.joabreu@synopsys.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Archit, On 23-06-2017 10:36, Jose Abreu wrote: > Currently HDMI 2.0 PHYs do not have a default configuration function. > > As *some* of the HDMI 2.0 PHYs have the same register layout as the 3D > PHYs we can provide the same default configuration function for both > and still let user overwrite this with custom configuration function > if needed. > > If, for some reason, the PHY is custom or has a register different > register layout then custom configuration function *must* be provided > in order for the system to work correctly. As we prefer the pdata > provided configuration function over the internal one this change > will not make any impact in custom platforms. > > This patch is based on today's drm-misc-next branch. > > Signed-off-by: Jose Abreu <joabreu@synopsys.com> > Tested-by: Mark Yao <mark.yao@rock-chips.com> This is needed for RK3399 support. Can you please apply it? Best regards, Jose Miguel Abreu > 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> > Cc: Heiko Stübner <heiko@sntech.de> > > Changes in v2: > - Rebased and refrased commit message > --- > > Hi All, > > There as been a little confusion about dw-hdmi phys so I will expand a little bit > here so that I can base my decision about this patch and why does it only works > in some platforms. > > First, if you read dw-hdmi.c code, you will see that there is an identification > register for the phy type being used. Unfortunatelly, this only states the phy type > and not the phy version. > > Second, we have many HDMI 2.0 phys (so, same phy type: 0xf3) but, as you may have guessed, > HW team decided to change regbank between some versions. > > Third and last, each phy in a SoC has unique characteristics, so each phy (event if > they are the same version) will have different PLL configuration parameters. > > Given all this I managed to conclude that Mark's phy is still an HDMI 2.0 phy but with > the same register layout as previous 3D PHY's. I found at least 2 phys with the same > register layout and only 1 phy which has a different layout, so I think majority wins > here and we should let the default configuration function for HDMI 2.0 phys be the same > one as the 3D. > > Short story: There is no way to correctly identify, at runtime, the phy version being > used by the controller so we can't provide a default configuration function. We can, > however assume that most of the HDMI 2.0 phys will have the 3D layout BUT each developer > must confirm that the layout in its SoC is the expected one and if not, provide a custom > configuration function. > > Best regards, > Jose Miguel Abreu > > 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 Friday 23 Jun 2017 10:36:44 Jose Abreu wrote: > Currently HDMI 2.0 PHYs do not have a default configuration function. > > As *some* of the HDMI 2.0 PHYs have the same register layout as the 3D > PHYs we can provide the same default configuration function for both > and still let user overwrite this with custom configuration function > if needed. > > If, for some reason, the PHY is custom or has a register different > register layout then custom configuration function *must* be provided > in order for the system to work correctly. As we prefer the pdata > provided configuration function over the internal one this change > will not make any impact in custom platforms. > > This patch is based on today's drm-misc-next branch. > > Signed-off-by: Jose Abreu <joabreu@synopsys.com> > Tested-by: Mark Yao <mark.yao@rock-chips.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> > Cc: Heiko Stübner <heiko@sntech.de> > > Changes in v2: > - Rebased and refrased commit message > --- > > Hi All, > > There as been a little confusion about dw-hdmi phys so I will expand a > little bit here so that I can base my decision about this patch and why > does it only works in some platforms. > > First, if you read dw-hdmi.c code, you will see that there is an > identification register for the phy type being used. Unfortunatelly, this > only states the phy type and not the phy version. > > Second, we have many HDMI 2.0 phys (so, same phy type: 0xf3) but, as you may > have guessed, HW team decided to change regbank between some versions. > > Third and last, each phy in a SoC has unique characteristics, so each phy > (event if they are the same version) will have different PLL configuration > parameters. > > Given all this I managed to conclude that Mark's phy is still an HDMI 2.0 > phy but with the same register layout as previous 3D PHY's. I found at > least 2 phys with the same register layout and only 1 phy which has a > different layout, so I think majority wins here and we should let the > default configuration function for HDMI 2.0 phys be the same one as the 3D. > > Short story: There is no way to correctly identify, at runtime, the phy > version being used by the controller so we can't provide a default > configuration function. :'-( > We can, however assume that most of the HDMI 2.0 phys will have the 3D > layout BUT each developer must confirm that the layout in its SoC is the > expected one and if not, provide a custom configuration function. This seems fair to me, so Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> However, if the PHY version can't be identified at runtime, I wonder whether it would make sense to add it to the PHY DT node. The driver could then pick the right configuration function accordingly. SoC-specific parameters (such as PLL parameters) would still be passed through platform glue code, but all the rest could be moved to the core driver. > 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",
On 2017年07月07日 23:47, Jose Abreu wrote: > Hi Archit, > Ping, Any update for this patch? Can it be landed? This patch actually needed for rk3399 hdmi support Best Regards Mark > On 23-06-2017 10:36, Jose Abreu wrote: >> Currently HDMI 2.0 PHYs do not have a default configuration function. >> >> As *some* of the HDMI 2.0 PHYs have the same register layout as the 3D >> PHYs we can provide the same default configuration function for both >> and still let user overwrite this with custom configuration function >> if needed. >> >> If, for some reason, the PHY is custom or has a register different >> register layout then custom configuration function *must* be provided >> in order for the system to work correctly. As we prefer the pdata >> provided configuration function over the internal one this change >> will not make any impact in custom platforms. >> >> This patch is based on today's drm-misc-next branch. >> >> Signed-off-by: Jose Abreu <joabreu@synopsys.com> >> Tested-by: Mark Yao <mark.yao@rock-chips.com> > This is needed for RK3399 support. Can you please apply it? > > Best regards, > Jose Miguel Abreu > >> 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> >> Cc: Heiko Stübner <heiko@sntech.de> >> >> Changes in v2: >> - Rebased and refrased commit message >> --- >> >> Hi All, >> >> There as been a little confusion about dw-hdmi phys so I will expand a little bit >> here so that I can base my decision about this patch and why does it only works >> in some platforms. >> >> First, if you read dw-hdmi.c code, you will see that there is an identification >> register for the phy type being used. Unfortunatelly, this only states the phy type >> and not the phy version. >> >> Second, we have many HDMI 2.0 phys (so, same phy type: 0xf3) but, as you may have guessed, >> HW team decided to change regbank between some versions. >> >> Third and last, each phy in a SoC has unique characteristics, so each phy (event if >> they are the same version) will have different PLL configuration parameters. >> >> Given all this I managed to conclude that Mark's phy is still an HDMI 2.0 phy but with >> the same register layout as previous 3D PHY's. I found at least 2 phys with the same >> register layout and only 1 phy which has a different layout, so I think majority wins >> here and we should let the default configuration function for HDMI 2.0 phys be the same >> one as the 3D. >> >> Short story: There is no way to correctly identify, at runtime, the phy version being >> used by the controller so we can't provide a default configuration function. We can, >> however assume that most of the HDMI 2.0 phys will have the 3D layout BUT each developer >> must confirm that the layout in its SoC is the expected one and if not, provide a custom >> configuration function. >> >> Best regards, >> Jose Miguel Abreu >> >> 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, On 08/08/2017 12:35 PM, Mark yao wrote: > On 2017年07月07日 23:47, Jose Abreu wrote: >> Hi Archit, >> > > Ping, Any update for this patch? Can it be landed? > > This patch actually needed for rk3399 hdmi support Sorry, missed this one. There was a comment from Laurent mentioning whether we'd want to put the PHY version in DT. There wasn't any response to that. However, it seems like this commit shouldn't hurt if we do decide to put PHY versions in DT later. Queuing it to drm-misc-next. Archit > > Best Regards > Mark > >> On 23-06-2017 10:36, Jose Abreu wrote: >>> Currently HDMI 2.0 PHYs do not have a default configuration function. >>> >>> As *some* of the HDMI 2.0 PHYs have the same register layout as the 3D >>> PHYs we can provide the same default configuration function for both >>> and still let user overwrite this with custom configuration function >>> if needed. >>> >>> If, for some reason, the PHY is custom or has a register different >>> register layout then custom configuration function *must* be provided >>> in order for the system to work correctly. As we prefer the pdata >>> provided configuration function over the internal one this change >>> will not make any impact in custom platforms. >>> >>> This patch is based on today's drm-misc-next branch. >>> >>> Signed-off-by: Jose Abreu <joabreu@synopsys.com> >>> Tested-by: Mark Yao <mark.yao@rock-chips.com> >> This is needed for RK3399 support. Can you please apply it? >> >> Best regards, >> Jose Miguel Abreu >> >>> 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> >>> Cc: Heiko Stübner <heiko@sntech.de> >>> >>> Changes in v2: >>> - Rebased and refrased commit message >>> --- >>> >>> Hi All, >>> >>> There as been a little confusion about dw-hdmi phys so I will expand a little bit >>> here so that I can base my decision about this patch and why does it only works >>> in some platforms. >>> >>> First, if you read dw-hdmi.c code, you will see that there is an identification >>> register for the phy type being used. Unfortunatelly, this only states the phy type >>> and not the phy version. >>> >>> Second, we have many HDMI 2.0 phys (so, same phy type: 0xf3) but, as you may have guessed, >>> HW team decided to change regbank between some versions. >>> >>> Third and last, each phy in a SoC has unique characteristics, so each phy (event if >>> they are the same version) will have different PLL configuration parameters. >>> >>> Given all this I managed to conclude that Mark's phy is still an HDMI 2.0 phy but with >>> the same register layout as previous 3D PHY's. I found at least 2 phys with the same >>> register layout and only 1 phy which has a different layout, so I think majority wins >>> here and we should let the default configuration function for HDMI 2.0 phys be the same >>> one as the 3D. >>> >>> Short story: There is no way to correctly identify, at runtime, the phy version being >>> used by the controller so we can't provide a default configuration function. We can, >>> however assume that most of the HDMI 2.0 phys will have the 3D layout BUT each developer >>> must confirm that the layout in its SoC is the expected one and if not, provide a custom >>> configuration function. >>> >>> Best regards, >>> Jose Miguel Abreu >>> >>> 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", >> >> >> >
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",