| Message ID | alpine.DEB.1.10.1103161531530.13431@cnc.isely.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Mike, I wasn't keen on introducing this option to disable the LVDS fixed mode. I think we want to allow the user to specify the fixed mode to use instead, without the intermediate stop-gap solution of completely disabling the probe. Later we can extend that ability (if possible or still desirable on reflection) so that we can specify all the LVDS configuration options in the single string. But first can you check that I've correctly forward ported your other patches on top of Eric's refactoring. Many thanks, -Chris
On Wed, 13 Apr 2011, Chris Wilson wrote: > Mike, I wasn't keen on introducing this option to disable the LVDS fixed > mode. I think we want to allow the user to specify the fixed mode to use > instead, without the intermediate stop-gap solution of completely disabling > the probe. Later we can extend that ability (if possible or still desirable > on reflection) so that we can specify all the LVDS configuration options > in the single string. Chris: Keep in mind that if the probe action is returning wrong results, that's worse than no results. This isn't a question about "supplying missing data" so much as it a case of "overriding bad data". Without that fixed mode disabling ability, it is very possible that the panel will simply be set up wrong with a corrupted display, with no possible means for the user to repair the problem. That's what led me to make the change, and that same change had been living peacefully in the userspace xorg driver, up until the point that UMS had been removed from it. Right now without the ability to disable fixed mode there simply *is no* workaround. Now with that said I understand where you're driving at here and if we can get a means to actually specify an independent fixed mode solution then that clearly also solves the problem and I agree it's more elegant. My only worry is the fact that the more elegant solution is also more complex may result in there being no solution implemented, which is even worse than the other two choices (disable fixed mode vs specifying fixed mode). I guess I just want to make sure that if the ability to disable fixed mode is not merged that there at least be continuous progress towards being able to set the fixed mode. And I'm willing to do whatever I can to help code and/or test there. > > But first can you check that I've correctly forward ported your other > patches on top of Eric's refactoring. Thanks. Will do. Stay tuned. -Mike
On Wed, 13 Apr 2011 02:27:18 -0500 (CDT), Mike Isely <isely@isely.net> wrote: > Keep in mind that if the probe action is returning wrong results, that's > worse than no results. This isn't a question about "supplying missing > data" so much as it a case of "overriding bad data". Without that fixed > mode disabling ability, it is very possible that the panel will simply > be set up wrong with a corrupted display, with no possible means for the > user to repair the problem. That's what led me to make the change, and > that same change had been living peacefully in the userspace xorg > driver, up until the point that UMS had been removed from it. Right now > without the ability to disable fixed mode there simply *is no* > workaround. Point taken. If we can't get a resolution on i915.lvds_fixed_mode= within the next week, let's focus on the workaround instead. -Chris
On Wed, 13 Apr 2011, Chris Wilson wrote: > On Wed, 13 Apr 2011 02:27:18 -0500 (CDT), Mike Isely <isely@isely.net> wrote: > > Keep in mind that if the probe action is returning wrong results, that's > > worse than no results. This isn't a question about "supplying missing > > data" so much as it a case of "overriding bad data". Without that fixed > > mode disabling ability, it is very possible that the panel will simply > > be set up wrong with a corrupted display, with no possible means for the > > user to repair the problem. That's what led me to make the change, and > > that same change had been living peacefully in the userspace xorg > > driver, up until the point that UMS had been removed from it. Right now > > without the ability to disable fixed mode there simply *is no* > > workaround. > > Point taken. If we can't get a resolution on i915.lvds_fixed_mode= within > the next week, let's focus on the workaround instead. > -Chris Sounds good. The remaining updated patches look good as well. -Mike isely@pobox.com isely@isely.net
different than the bad old days of CRT video modes. And while we normally don't want to disable fixed mode, the fact of the matter is that sometimes there's no choice if the driver otherwise gets the fixed mode "wrong". This can happen in some embedded systems where the video BIOS really has no knowledge of the display device. Warning: With fixed mode disabled, and if there is no other mode data supplied anywhere (e.g. KMS trying to run a framebuffer for fbcon on an LVDS console), then the LVDS port will likely simply be disabled. However that's easily remedied by supplying video mode information on the kernel command line, for example "video=LVDS-1:800x600-24MR@100" for a 100Hz 800x600 24bpp resolution. Signed-off-by: Mike Isely <isely@pobox.com> --- drivers/gpu/drm/i915/i915_drv.c | 3 +++ drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_lvds.c | 31 ++++++++++++++++++++++--------- 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index c89f71c251acf230db613229eca90d24584b9729..004880aa3a948669b8b4e23d9ad73d132cff81d0 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -46,6 +46,9 @@ module_param_named(fbpercrtc, i915_fbpercrtc, int, 0400); unsigned int i915_powersave = 1; module_param_named(powersave, i915_powersave, int, 0600); +unsigned int i915_lvds_fixed = 1; +module_param_named(lvds_fixed, i915_lvds_fixed, int, 0600); + unsigned int i915_lvds_downclock = 0; module_param_named(lvds_downclock, i915_lvds_downclock, int, 0400); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 526cef2972ab9afce1c17f57ef39ce9cc4dc736f..3fa8681459aa596e12e885568e5b48f0c9a60719 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -885,6 +885,7 @@ extern struct drm_ioctl_desc i915_ioctls[]; extern int i915_max_ioctl; extern unsigned int i915_fbpercrtc; extern unsigned int i915_powersave; +extern unsigned int i915_lvds_fixed; extern unsigned int i915_lvds_downclock; extern unsigned int i915_lvds_24bit; diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c index fe779b33899af536eb2e76429c9b05c363ce7721..303d60f71ca6dcf4ce7b59fe625ed5377af24bc4 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -157,10 +157,12 @@ static int intel_lvds_mode_valid(struct drm_connector *connector, struct intel_lvds *intel_lvds = intel_attached_lvds(connector); struct drm_display_mode *fixed_mode = intel_lvds->fixed_mode; - if (mode->hdisplay > fixed_mode->hdisplay) - return MODE_PANEL; - if (mode->vdisplay > fixed_mode->vdisplay) - return MODE_PANEL; + if (fixed_mode) { + if (mode->hdisplay > fixed_mode->hdisplay) + return MODE_PANEL; + if (mode->vdisplay > fixed_mode->vdisplay) + return MODE_PANEL; + } return MODE_OK; } @@ -253,7 +255,8 @@ static bool intel_lvds_mode_fixup(struct drm_encoder *encoder, * with the panel scaling set up to source from the H/VDisplay * of the original mode. */ - intel_fixed_panel_mode(intel_lvds->fixed_mode, adjusted_mode); + if (intel_lvds->fixed_mode) + intel_fixed_panel_mode(intel_lvds->fixed_mode, adjusted_mode); if (HAS_PCH_SPLIT(dev)) { intel_pch_panel_fitting(dev, intel_lvds->fitting_mode, @@ -488,11 +491,13 @@ static int intel_lvds_get_modes(struct drm_connector *connector) if (intel_lvds->edid) return drm_add_edid_modes(connector, intel_lvds->edid); - mode = drm_mode_duplicate(dev, intel_lvds->fixed_mode); - if (mode == 0) - return 0; + if (intel_lvds->fixed_mode) { + mode = drm_mode_duplicate(dev, intel_lvds->fixed_mode); + if (mode == 0) + return 0; - drm_mode_probed_add(connector, mode); + drm_mode_probed_add(connector, mode); + } return 1; } @@ -942,6 +947,14 @@ bool intel_lvds_init(struct drm_device *dev) * if closed, act like it's not there for now */ + if (!i915_lvds_fixed) { + DRM_DEBUG_KMS("Skipping any attempt to determine" + " panel fixed mode.\n"); + goto out; + } + DRM_DEBUG_KMS("Attempting to determine panel fixed mode.\n"); + + /* * Attempt to get the fixed panel mode from DDC. Assume that the * preferred mode is the right one.