| Message ID | 1401992671-2548-4-git-send-email-jbarnes@virtuousgeek.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jun 05, 2014 at 11:24:30AM -0700, Jesse Barnes wrote: > From: Kristian Høgsberg <hoegsberg@gmail.com> > > The BIOS may set a native mode that doesn't quite match the preferred > mode timings. It should be ok to use however if it uses the same size, > so try to avoid a mode set in that case. > > Signed-off-by: Kristian Høgsberg <hoegsberg@gmail.com> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> Not sure we want this since this seems to override the cmdline options to force a specific edid. Also not sure whether we shouldn't just add this as the preferred mode when probing (before the preferred mode the vbt/edid provides ofc). What exactly is the mismatch here? It could be DRRS or something fancy, too. Not sure what to do here really. -Daniel > --- > drivers/gpu/drm/i915/intel_fbdev.c | 47 +++++++++++++++----------------------- > 1 file changed, 18 insertions(+), 29 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c > index 088fe93..09819ae 100644 > --- a/drivers/gpu/drm/i915/intel_fbdev.c > +++ b/drivers/gpu/drm/i915/intel_fbdev.c > @@ -379,42 +379,31 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper, > /* go for command line mode first */ > modes[i] = drm_pick_cmdline_mode(fb_conn, width, height); > > - /* try for preferred next */ > + /* try for preferred next or match current */ > if (!modes[i]) { > - DRM_DEBUG_KMS("looking for preferred mode on connector %s\n", > - connector->name); > - modes[i] = drm_has_preferred_mode(fb_conn, width, > - height); > - } > + struct drm_display_mode *preferred; > > - /* No preferred mode marked by the EDID? Are there any modes? */ > - if (!modes[i] && !list_empty(&connector->modes)) { > - DRM_DEBUG_KMS("using first mode listed on connector %s\n", > + DRM_DEBUG_KMS("looking for preferred mode on connector %s\n", > connector->name); > - modes[i] = list_first_entry(&connector->modes, > - struct drm_display_mode, > - head); > - } > + preferred = drm_has_preferred_mode(fb_conn, width, > + height); > > - /* last resort: use current mode */ > - if (!modes[i]) { > - /* > - * IMPORTANT: We want to use the adjusted mode (i.e. > - * after the panel fitter upscaling) as the initial > - * config, not the input mode, which is what crtc->mode > - * usually contains. But since our current fastboot > - * code puts a mode derived from the post-pfit timings > - * into crtc->mode this works out correctly. We don't > - * use hwmode anywhere right now, so use it for this > - * since the fb helper layer wants a pointer to > - * something we own. > - */ > - DRM_DEBUG_KMS("looking for current mode on connector %s\n", > - connector->name); > intel_mode_from_pipe_config(&encoder->crtc->hwmode, > &to_intel_crtc(encoder->crtc)->config); > - modes[i] = &encoder->crtc->hwmode; > + modes[i] = &encoder->crtc->hwmode; > + > + if (preferred && > + !drm_mode_same_size(preferred, modes[i])) { > + DRM_DEBUG_KMS("using preferred mode %s " > + "instead of current mode %s " > + "on connector %d\n", > + preferred->name, > + modes[i]->name, > + fb_conn->connector->base.id); > + modes[i] = preferred; > + } > } > + > crtcs[i] = new_crtc; > > DRM_DEBUG_KMS("connector %s on pipe %d [CRTC:%d]: %dx%d%s\n", > -- > 1.8.3.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, 10 Jun 2014 16:05:36 +0200 Daniel Vetter <daniel@ffwll.ch> wrote: > On Thu, Jun 05, 2014 at 11:24:30AM -0700, Jesse Barnes wrote: > > From: Kristian Høgsberg <hoegsberg@gmail.com> > > > > The BIOS may set a native mode that doesn't quite match the preferred > > mode timings. It should be ok to use however if it uses the same size, > > so try to avoid a mode set in that case. > > > > Signed-off-by: Kristian Høgsberg <hoegsberg@gmail.com> > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > Not sure we want this since this seems to override the cmdline options to > force a specific edid. Also not sure whether we shouldn't just add this as > the preferred mode when probing (before the preferred mode the vbt/edid > provides ofc). > > What exactly is the mismatch here? It could be DRRS or something fancy, > too. > > Not sure what to do here really. AFAICT it's just slightly different timings for fun. I don't think they go low enough to reduce the DP lane count... maybe there's just a mismatch between their hard coded panel timings and the ones reported by the EDID. Not sure which to trust... Kristian can you post the timings you see here? Both the BIOS timings and the EDID ones? So I'm stuck here too, I think it's a rare case though.
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index 088fe93..09819ae 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -379,42 +379,31 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper, /* go for command line mode first */ modes[i] = drm_pick_cmdline_mode(fb_conn, width, height); - /* try for preferred next */ + /* try for preferred next or match current */ if (!modes[i]) { - DRM_DEBUG_KMS("looking for preferred mode on connector %s\n", - connector->name); - modes[i] = drm_has_preferred_mode(fb_conn, width, - height); - } + struct drm_display_mode *preferred; - /* No preferred mode marked by the EDID? Are there any modes? */ - if (!modes[i] && !list_empty(&connector->modes)) { - DRM_DEBUG_KMS("using first mode listed on connector %s\n", + DRM_DEBUG_KMS("looking for preferred mode on connector %s\n", connector->name); - modes[i] = list_first_entry(&connector->modes, - struct drm_display_mode, - head); - } + preferred = drm_has_preferred_mode(fb_conn, width, + height); - /* last resort: use current mode */ - if (!modes[i]) { - /* - * IMPORTANT: We want to use the adjusted mode (i.e. - * after the panel fitter upscaling) as the initial - * config, not the input mode, which is what crtc->mode - * usually contains. But since our current fastboot - * code puts a mode derived from the post-pfit timings - * into crtc->mode this works out correctly. We don't - * use hwmode anywhere right now, so use it for this - * since the fb helper layer wants a pointer to - * something we own. - */ - DRM_DEBUG_KMS("looking for current mode on connector %s\n", - connector->name); intel_mode_from_pipe_config(&encoder->crtc->hwmode, &to_intel_crtc(encoder->crtc)->config); - modes[i] = &encoder->crtc->hwmode; + modes[i] = &encoder->crtc->hwmode; + + if (preferred && + !drm_mode_same_size(preferred, modes[i])) { + DRM_DEBUG_KMS("using preferred mode %s " + "instead of current mode %s " + "on connector %d\n", + preferred->name, + modes[i]->name, + fb_conn->connector->base.id); + modes[i] = preferred; + } } + crtcs[i] = new_crtc; DRM_DEBUG_KMS("connector %s on pipe %d [CRTC:%d]: %dx%d%s\n",