Hi, On 6/1/22 10:18, Hsin-Yi Wang 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@xxxxxxxxxxxx> > --- > The concept is the same as the previous version. > https://patchwork.kernel.org/project/linux-mediatek/patch/20220530113033.124072-1-hsinyi@xxxxxxxxxxxx/ > The only difference is, it now uses the panel API instead of parsing > orientation from the driver. > --- > drivers/gpu/drm/mediatek/mtk_dsi.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c > index bd3f5b485085..12836a697f56 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,16 @@ 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 */ > + if (dsi->panel) { > + enum drm_panel_orientation orientation; > + > + orientation = drm_panel_get_orientation(dsi->panel); > + if (orientation != DRM_MODE_PANEL_ORIENTATION_UNKNOWN) > + drm_connector_set_panel_orientation(dsi->connector, orientation); > + } > + > drm_connector_attach_encoder(dsi->connector, &dsi->encoder); > > return 0; drm_connector_set_panel_orientation() is a no-op when called with DRM_MODE_PANEL_ORIENTATION_UNKNOWN, so the check for this is not necessary. This allows this to be simplified to: /* Read panel orientation */ if (dsi->panel) drm_connector_set_panel_orientation(dsi->connector, drm_panel_get_orientation(dsi->panel)); Note since drm_panel_get_orientation() checks for a NULL panel, you could even drop the "if (dsi->panel)", but I think the meaning of the code is more clear with that present. > @@ -837,6 +848,9 @@ static int mtk_dsi_bind(struct device *dev, struct device *master, void *data) > struct drm_device *drm = data; > struct mtk_dsi *dsi = dev_get_drvdata(dev); > > + /* Get panel if existed */ > + ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0, &dsi->panel, NULL); > + Check ret? or maybe not assign to ret ? I understand some errors are expected so maybe something like: if (ret && ret != -ENODEV) return ret; ? Note -ENODEV is probably not the right error the check for! Regards, Hans > ret = mtk_dsi_encoder_init(drm, dsi); > if (ret) > return ret;
