Re: [PATCH v5 8/8] drm/mediatek: Config orientation property if panel provides it

Hi,

On Tue, Jun 7, 2022 at 2:06 AM Hsin-Yi Wang hsinyi@chromium.org wrote:

Panel orientation property should be set before drm_dev_register(). Mediatek drm driver calls drm_dev_register() in .bind(). However, most panels sets orientation property relatively late, mostly in .get_modes() callback, since this is when they are able to get the connector and binds the orientation property to it, though the value should be known when the panel is probed.

Let the drm driver check if the remote end point is a panel and if it contains the orientation property. If it does, set it before drm_dev_register() is called.

Signed-off-by: Hsin-Yi Wang hsinyi@chromium.org Reviewed-by: Hans de Goede hdegoede@redhat.com Reviewed-by: AngeloGioacchino Del Regno angelogioacchino.delregno@collabora.com

---

v4->v5:

• use the new function in v5.
• don't use drm_of_find_panel_or_bridge().

---

drivers/gpu/drm/mediatek/mtk_dsi.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index d9f10a33e6fa..998b1237e193 100644 --- a/drivers/gpu/drm/mediatek/mtk_dsi.c +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c @@ -185,6 +185,7 @@ struct mtk_dsi { struct drm_encoder encoder; struct drm_bridge bridge; struct drm_bridge *next_bridge;

•   struct drm_panel *panel;
    struct drm_connector *connector;
    struct phy *phy;
  
  

@@ -822,6 +823,10 @@ static int mtk_dsi_encoder_init(struct drm_device *drm, struct mtk_dsi *dsi) ret = PTR_ERR(dsi->connector); goto err_cleanup_encoder; }

•   /* Read panel orientation */
  

•   drm_connector_set_orientation_from_panel(dsi->connector, dsi->panel);
  

•   drm_connector_attach_encoder(dsi->connector, &dsi->encoder);
  
    return 0;
  

@@ -836,6 +841,16 @@ static int mtk_dsi_bind(struct device *dev, struct device *master, void *data) int ret; struct drm_device *drm = data; struct mtk_dsi *dsi = dev_get_drvdata(dev);

•   struct device_node *panel_node;
  

•   /* Get panel if existed */
  

•   panel_node = of_graph_get_remote_node(dev->of_node, 0, 0);
  

•   if (panel_node) {
  

•           dsi->panel = of_drm_find_panel(panel_node);
  

•           if (IS_ERR(dsi->panel))
  

•                   dsi->panel = NULL;
  

•           of_node_put(panel_node);
  

•   }
  

While the above works, it feels like we could do better. What about this?

* We add _some_ way to determine if a bridge is actually a panel_bridge. If nothing else maybe this could be drm_bridge_is_panel() and it could just check if bridge.funcs == panel_bridge_bridge_funcs

* In drm_bridge_connector_init(), when we're looping through all the bridges we find the panel_bridge if it's there.

* At the end of drm_bridge_connector_init() if we found a panel_bridge then we call a function like drm_panel_bridge_set_orientation().

Then you can fully get rid of the mediatek patch, right? The above will only work if you're using a panel_bridge / bridge_connector, but that's "the future" anyway and we want to encourage people to transition to that.

-Doug