Re: [PATCH v3 3/4] drm/i915/edp: Allow alternate fixed mode for eDP if available.

[Jim Bride <jim.bride@linux.intel.com>]

On Wed, Jul 12, 2017 at 12:27:33AM +0100, Chris Wilson wrote:
> Quoting Jim Bride (2017-07-11 23:19:55)
> > @@ -5869,13 +5891,14 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> >         }
> >         intel_connector->edid = edid;
> >  
> > -       /* prefer fixed mode from EDID if available */
> > +       /* prefer fixed mode from EDID if available, save an alt mode also */
> >         list_for_each_entry(scan, &connector->probed_modes, head) {
> >                 if ((scan->type & DRM_MODE_TYPE_PREFERRED)) {
> >                         fixed_mode = drm_mode_duplicate(dev, scan);
> >                         downclock_mode = intel_dp_drrs_init(
> >                                                 intel_connector, fixed_mode);
> > -                       break;
> > +               } else {
> > +                       alt_fixed_mode = drm_mode_duplicate(dev, scan);
> >                 }
> >         }
> 
> Save one, leak the rest?

Good catch, I'll tweak the code so that we only duplicate once,
although in practice (on the panels I have to test with) fixed
panels support either one or two modes, with only the frame
rate being different if two are supported.

> > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> > index 96c2cbd..4e7ba93 100644
> > --- a/drivers/gpu/drm/i915/intel_panel.c
> > +++ b/drivers/gpu/drm/i915/intel_panel.c
> > @@ -1919,11 +1919,13 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel)
> >  
> >  int intel_panel_init(struct intel_panel *panel,
> >                      struct drm_display_mode *fixed_mode,
> > +                    struct drm_display_mode *alt_fixed_mode,
> >                      struct drm_display_mode *downclock_mode)
> >  {
> >         intel_panel_init_backlight_funcs(panel);
> >  
> >         panel->fixed_mode = fixed_mode;
> > +       panel->alt_fixed_mode = alt_fixed_mode;
> >         panel->downclock_mode = downclock_mode;
> 
> Ah, at least it is consistent; they all get leaked. :)

Yeah, I missed adding a destroy call in intel_panel_fini().  I'll
fix that.

Thanks for the review!

Jim


> In general though, it looks like for edp you want to scan
> connector->probed_modes at config time looking for an exact match in
> preference to using the scaled mode.
> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx