On 09/01/2024 21:04, Jagan Teki wrote:
Hi Neil,
On Tue, Dec 19, 2023 at 5:21 PM Jagan Teki <ja...@amarulasolutions.com> wrote:
On Tue, Dec 19, 2023 at 2:34 PM Neil Armstrong
<neil.armstr...@linaro.org> wrote:
On 18/12/2023 20:10, Jagan Teki wrote:
From: Jagan Teki <ja...@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 <ja...@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?
Let me know if you have any comments on this. I'm sending V3 would
probably hit in MW.
Sure, I understand your point, but there's a limited number of users and it can
be greatly simplified, no need to keep the current structure, there's no
external
users of the driver.
Just set hdmi->ops with the vendor phy ops from the glue driver if needed,
or let it to NULL if the platform uses the synopsys PHY, it's as simple as that.
If hdmi->ops is NULL it would be replaced with dw_hdmi_synopsys_phy_ops in the
dw_hdmi_detect_phy() function.
Neil
Thanks,
Jagan.
