| Message ID | 20231106202718.2770821-4-hsinyi@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add a few panels and use correct modes | expand |
Hi, On Mon, Nov 6, 2023 at 12:27 PM Hsin-Yi Wang <hsinyi@chromium.org> wrote: > > diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c > index 0fb439b5efb1..54dbbdf62ec0 100644 > --- a/drivers/gpu/drm/panel/panel-edp.c > +++ b/drivers/gpu/drm/panel/panel-edp.c > @@ -594,8 +594,20 @@ static int panel_edp_get_modes(struct drm_panel *panel, > p->detected_panel != ERR_PTR(-EINVAL) && > p->detected_panel->override_edid_mode; > > - /* probe EDID if a DDC bus is available */ > - if (p->ddc) { > + /* > + * If both edid and hard-coded modes exists, only add hard-coded modes > + * to avoid multiple preferred modes. > + */ > + if (p->desc->num_timings || p->desc->num_modes) { > + /* > + * Add hard-coded panel modes. Don't call this if there are no > + * timings and no modes (the generic edp-panel case) because it > + * will clobber the display_info that was already set by > + * drm_add_edid_modes(). > + */ > + num += panel_edp_get_non_edid_modes(p, connector); > + } else if (p->ddc) { > + /* probe EDID if a DDC bus is available */ As per discussion in v2, I think if you have the "ddc" you still want to fetch the EDID, you just don't want to add the modes from the EDID. This will mean that the EDID is present in sysfs if userspace wants to look at it for whatever reason. -Doug
On Mon, Nov 6, 2023 at 12:33 PM Doug Anderson <dianders@chromium.org> wrote: > > Hi, > > On Mon, Nov 6, 2023 at 12:27 PM Hsin-Yi Wang <hsinyi@chromium.org> wrote: > > > > diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c > > index 0fb439b5efb1..54dbbdf62ec0 100644 > > --- a/drivers/gpu/drm/panel/panel-edp.c > > +++ b/drivers/gpu/drm/panel/panel-edp.c > > @@ -594,8 +594,20 @@ static int panel_edp_get_modes(struct drm_panel *panel, > > p->detected_panel != ERR_PTR(-EINVAL) && > > p->detected_panel->override_edid_mode; > > > > - /* probe EDID if a DDC bus is available */ > > - if (p->ddc) { > > + /* > > + * If both edid and hard-coded modes exists, only add hard-coded modes > > + * to avoid multiple preferred modes. > > + */ > > + if (p->desc->num_timings || p->desc->num_modes) { > > + /* > > + * Add hard-coded panel modes. Don't call this if there are no > > + * timings and no modes (the generic edp-panel case) because it > > + * will clobber the display_info that was already set by > > + * drm_add_edid_modes(). > > + */ > > + num += panel_edp_get_non_edid_modes(p, connector); > > + } else if (p->ddc) { > > + /* probe EDID if a DDC bus is available */ > > As per discussion in v2, I think if you have the "ddc" you still want > to fetch the EDID, you just don't want to add the modes from the EDID. > This will mean that the EDID is present in sysfs if userspace wants to > look at it for whatever reason. > Ack. Will update this. > -Doug
diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c index 0fb439b5efb1..54dbbdf62ec0 100644 --- a/drivers/gpu/drm/panel/panel-edp.c +++ b/drivers/gpu/drm/panel/panel-edp.c @@ -594,8 +594,20 @@ static int panel_edp_get_modes(struct drm_panel *panel, p->detected_panel != ERR_PTR(-EINVAL) && p->detected_panel->override_edid_mode; - /* probe EDID if a DDC bus is available */ - if (p->ddc) { + /* + * If both edid and hard-coded modes exists, only add hard-coded modes + * to avoid multiple preferred modes. + */ + if (p->desc->num_timings || p->desc->num_modes) { + /* + * Add hard-coded panel modes. Don't call this if there are no + * timings and no modes (the generic edp-panel case) because it + * will clobber the display_info that was already set by + * drm_add_edid_modes(). + */ + num += panel_edp_get_non_edid_modes(p, connector); + } else if (p->ddc) { + /* probe EDID if a DDC bus is available */ pm_runtime_get_sync(panel->dev); if (!p->edid) @@ -617,14 +629,7 @@ static int panel_edp_get_modes(struct drm_panel *panel, pm_runtime_put_autosuspend(panel->dev); } - /* - * Add hard-coded panel modes. Don't call this if there are no timings - * and no modes (the generic edp-panel case) because it will clobber - * the display_info that was already set by drm_add_edid_modes(). - */ - if (p->desc->num_timings || p->desc->num_modes) - num += panel_edp_get_non_edid_modes(p, connector); - else if (!num) + if (!num) dev_warn(p->base.dev, "No display modes\n"); /*
If a non generic edp-panel is under aux-bus, the mode read from edid would still be selected as preferred and results in multiple preferred modes, which is ambiguous. If both hard-coded mode and edid exists, only add mode from hard-coded. Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org> --- v2->v3: if hard-coded mode presents, don't add edid mode. --- drivers/gpu/drm/panel/panel-edp.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-)