Hi Laurent, Am Montag, 19. Februar 2018, 17:59:02 CET schrieb Laurent Pinchart: > On Friday, 16 February 2018 22:41:53 EET Heiko Stuebner wrote: > > In some IP implementations the reading of the phy-type may be broken. > > One example are the Rockchip rk3228 and rk3328 socs that use a separate > > phy from Innosilicon but still report the HDMI20_TX type. > > > > So allow the glue driver to force a specific type, like the vendor-phy > > for these cases. > > > > Signed-off-by: Heiko Stuebner <heiko@xxxxxxxxx> > > Tested-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> thanks for testing :-) > > --- > > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 4 +++- > > include/drm/bridge/dw_hdmi.h | 1 + > > 2 files changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index > > f9802399cc0d..50d231626c4d 100644 > > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > > @@ -2218,7 +2218,9 @@ static int dw_hdmi_detect_phy(struct dw_hdmi *hdmi) > > unsigned int i; > > u8 phy_type; > > > > - phy_type = hdmi_readb(hdmi, HDMI_CONFIG2_ID); > > + phy_type = (hdmi->plat_data->phy_force_type) ? > > + hdmi->plat_data->phy_force_type : > > + hdmi_readb(hdmi, HDMI_CONFIG2_ID); > > No need for parentheses. You could even write this > > phy_type = hdmi->plat_data->phy_force_type ? > : hdmi_readb(hdmi, HDMI_CONFIG2_ID); > > but that's up to you. > > What if a driver wants to force the PHY type to DW_HDMI_PHY_DWC_HDMI_TX_PHY ? > Or do you expect only the DW_HDMI_PHY_VENDOR_PHY type to be forced ? If so, we > could also use a force_vendor_phy boolean field instead of phy_force_type. Initially I thought of implicitly overriding the phy-type when the external phy-ops are set, but decided against it because that might break (theoretical) cases where phy-ops may be always set but only used when the ip returns a matching phy-type and thus came to just allow overriding that type reading. As for limiting to only allow forcing the vendor type, my personal feeling would be to allow glue drivers to just override the type as needed like done in the patch. As can be seen on the rk3328, the dw-hdmi reports one of the regular phys (forgot which one) but instead has a completely separate ip for it, so I'd guess we may very well see implementations that just report a wrong type (no vendor-type). So I don't see an issue with drivers being allowed to set the type to DW_HDMI_PHY_DWC_HDMI_TX_PHY, because such cases may exist in the future and I'd expect driver authors to somewhat know what they're doing. But I'm not tied to that, so will go with the majority opinion :-D . Heiko -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
