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

Jim Bride
2017-07-12T20:01:41Z

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