On Tue, Dec 19, 2023 at 2:34 PM Neil Armstrong neil.armstrong@linaro.org wrote:
On 18/12/2023 20:10, Jagan Teki wrote:
From: Jagan Teki jagan@edgeble.ai
DW HDMI support Vendor PHY like Rockchip RK3328 Inno HDMI PHY.
Extend the vendor phy handling by adding platform phy hooks.
Signed-off-by: Jagan Teki jagan@edgeble.ai
Changes for v2:
fix meson cfg
drivers/video/dw_hdmi.c | 29 +++++++++++++++++++++++++++- drivers/video/meson/meson_dw_hdmi.c | 11 ++++++++++- drivers/video/rockchip/rk3399_hdmi.c | 8 +++++++- drivers/video/rockchip/rk_hdmi.c | 2 +- drivers/video/sunxi/sunxi_dw_hdmi.c | 11 ++++++++++- include/dw_hdmi.h | 14 +++++++++++++- 6 files changed, 69 insertions(+), 6 deletions(-)
diff --git a/drivers/video/dw_hdmi.c b/drivers/video/dw_hdmi.c index c4fbb18294..ea12a09407 100644 --- a/drivers/video/dw_hdmi.c +++ b/drivers/video/dw_hdmi.c @@ -988,7 +988,7 @@ int dw_hdmi_enable(struct dw_hdmi *hdmi, const struct display_timing *edid)
hdmi_av_composer(hdmi, edid);
ret = hdmi->phy_set(hdmi, edid->pixelclock.typ);
ret = hdmi->ops->phy_set(hdmi, edid->pixelclock.typ); if (ret) return ret;@@ -1009,10 +1009,37 @@ int dw_hdmi_enable(struct dw_hdmi *hdmi, const struct display_timing *edid) return 0; }
+static const struct dw_hdmi_phy_ops dw_hdmi_synopsys_phy_ops = {
.phy_set = dw_hdmi_phy_cfg,+};
+static void dw_hdmi_detect_phy(struct dw_hdmi *hdmi) +{
if (!hdmi->data)return;/* hook Synopsys PHYs ops */if (!hdmi->data->phy_force_vendor) {hdmi->ops = &dw_hdmi_synopsys_phy_ops;return;}/* Vendor HDMI PHYs must assign phy_ops in plat_data */if (!hdmi->data->phy_ops) {printf("Unsupported Vendor HDMI phy_ops\n");return;}/* hook Vendor HDMI PHYs ops */hdmi->ops = hdmi->data->phy_ops;Sorry but I still don't understand why you need phy_force_vendor & phy_ops, this code clearly fails if you have phy_force_vendor=true && phy_ops=NULL, so drop phy_force_vendor and simply use phy_ops if != NULL, and since it's the only element of dw_hdmi_plat_data, drop dw_hdmi_plat_data and pass dw_hdmi_phy_ops directly in the dw_hdmi struct.
So in dw_hdmi_detect_phy(), if hdmi->ops is NULL, set it to dw_hdmi_synopsys_phy_ops.
Let me elaborate more.
DW HDMI IP must have phy ops. It never be NULL. Either it uses 1. Internal PHY via DW called them Synopsys PHYs ops - for example, rk3399 2. Vendor PHY via vendor phy meson, sunxi, rk3328
For case 1) phy_force_vendor is false so it uses dw_hdmi_synopsys_phy_ops For case 2) phy_force_vendor is true so it uses dw_hdmi_plat_data phy ops
dw_hdmi_detect_phy assigns internal phy ops first and then vendor phy ops based on phy_force_vendor flag.
If we remove dw_hdmi_plat_data how can we assign or differentiate two types of phy ops hooks? can you explain?
Thanks, Jagan.