diff mbox series

[v3,3/3] drm/panel-edp: Avoid adding multiple preferred modes

Message ID 20231106202718.2770821-4-hsinyi@chromium.org
State New
Headers
Series Add a few panels and use correct modes |

Commit Message

Hsin-Yi Wang Nov. 6, 2023, 8:22 p.m. UTC 
  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(-)

Comments

Doug Anderson Nov. 6, 2023, 8:32 p.m. UTC | #1 
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
Hsin-Yi Wang Nov. 6, 2023, 8:38 p.m. UTC | #2 
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 mbox series

Patch

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");
 
 	/*