| Message ID | 20240716121112.14435-3-lvzhaoxiong@huaqin.corp-partner.google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Modify the method of obtaining porch parameters | expand |
Hi, On Tue, Jul 16, 2024 at 5:11 AM Zhaoxiong Lv <lvzhaoxiong@huaqin.corp-partner.google.com> wrote: > > Use public functions(drm_connector_helper_get_modes_fixed()) to > get porch parameters. > > Signed-off-by: Zhaoxiong Lv <lvzhaoxiong@huaqin.corp-partner.google.com> > --- > .../drm/panel/panel-boe-th101mb31ig002-28a.c | 26 ++----------------- > 1 file changed, 2 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/panel/panel-boe-th101mb31ig002-28a.c b/drivers/gpu/drm/panel/panel-boe-th101mb31ig002-28a.c > index d4e4abd103bb..4a61a11099b6 100644 > --- a/drivers/gpu/drm/panel/panel-boe-th101mb31ig002-28a.c > +++ b/drivers/gpu/drm/panel/panel-boe-th101mb31ig002-28a.c > @@ -16,6 +16,7 @@ > #include <drm/drm_mipi_dsi.h> > #include <drm/drm_modes.h> > #include <drm/drm_panel.h> > +#include <drm/drm_probe_helper.h> > > struct boe_th101mb31ig002; > > @@ -313,31 +314,8 @@ static int boe_th101mb31ig002_get_modes(struct drm_panel *panel, > struct boe_th101mb31ig002, > panel); > const struct drm_display_mode *desc_mode = ctx->desc->modes; > - struct drm_display_mode *mode; > > - mode = drm_mode_duplicate(connector->dev, desc_mode); > - if (!mode) { > - dev_err(panel->dev, "Failed to add mode %ux%u@%u\n", > - desc_mode->hdisplay, desc_mode->vdisplay, > - drm_mode_vrefresh(desc_mode)); > - return -ENOMEM; > - } > - > - drm_mode_set_name(mode); > - > - connector->display_info.bpc = 8; I notice that drm_connector_helper_get_modes_fixed() doesn't seem to set bpc. Unless I'm mistaken and that gets set automatically somewhere else then you should keep that, right? > - connector->display_info.width_mm = mode->width_mm; > - connector->display_info.height_mm = mode->height_mm; > - > - /* > - * TODO: Remove once all drm drivers call > - * drm_connector_set_orientation_from_panel() > - */ > - drm_connector_set_panel_orientation(connector, ctx->orientation); Are we confident that all the other users of this panel are properly getting the orientation and we can remove the above bit of code? It looks like one other user is 'rk3566-pinetab2'. From what I recall, the relevant commits are commit 15b9ca1641f0 ("drm: Config orientation property if panel provides it") and commit e3ea1806e4ad ("drm/bridge: panel: Set orientation on panel_bridge connector"). I think in all cases the assumption was that, to get the right functionality we need to switch to "panel_bridge". That happens when we use drmm_of_get_bridge() or devm_drm_of_get_bridge(). ...but it looks like Rockchip DRM is directly using drm_of_find_panel_or_bridge() and thus hasn't switched to panel bridge. ...so, unless I'm mistaken, the other users of this panel driver still need the drm_connector_set_panel_orientation() call here and you shouldn't remove it. Perhaps Alexander Warnecke could comment about whether this is still needed. ...or perhaps someone who maintains Rockchip DRM can say whether they have any plans around this area? If, for some reason, you do remove it then it should at least be called out in the description since this is a functionality change. -Doug
diff --git a/drivers/gpu/drm/panel/panel-boe-th101mb31ig002-28a.c b/drivers/gpu/drm/panel/panel-boe-th101mb31ig002-28a.c index d4e4abd103bb..4a61a11099b6 100644 --- a/drivers/gpu/drm/panel/panel-boe-th101mb31ig002-28a.c +++ b/drivers/gpu/drm/panel/panel-boe-th101mb31ig002-28a.c @@ -16,6 +16,7 @@ #include <drm/drm_mipi_dsi.h> #include <drm/drm_modes.h> #include <drm/drm_panel.h> +#include <drm/drm_probe_helper.h> struct boe_th101mb31ig002; @@ -313,31 +314,8 @@ static int boe_th101mb31ig002_get_modes(struct drm_panel *panel, struct boe_th101mb31ig002, panel); const struct drm_display_mode *desc_mode = ctx->desc->modes; - struct drm_display_mode *mode; - mode = drm_mode_duplicate(connector->dev, desc_mode); - if (!mode) { - dev_err(panel->dev, "Failed to add mode %ux%u@%u\n", - desc_mode->hdisplay, desc_mode->vdisplay, - drm_mode_vrefresh(desc_mode)); - return -ENOMEM; - } - - drm_mode_set_name(mode); - - connector->display_info.bpc = 8; - connector->display_info.width_mm = mode->width_mm; - connector->display_info.height_mm = mode->height_mm; - - /* - * TODO: Remove once all drm drivers call - * drm_connector_set_orientation_from_panel() - */ - drm_connector_set_panel_orientation(connector, ctx->orientation); - - drm_mode_probed_add(connector, mode); - - return 1; + return drm_connector_helper_get_modes_fixed(connector, desc_mode); } static enum drm_panel_orientation
Use public functions(drm_connector_helper_get_modes_fixed()) to get porch parameters. Signed-off-by: Zhaoxiong Lv <lvzhaoxiong@huaqin.corp-partner.google.com> --- .../drm/panel/panel-boe-th101mb31ig002-28a.c | 26 ++----------------- 1 file changed, 2 insertions(+), 24 deletions(-)