[Intel-gfx] [PATCH 0/3] [drm/i915] - LVDS mode setting fixes

[Mike Isely]

On Thu, 17 Mar 2011, Chris Wilson wrote:

> On Thu, 17 Mar 2011 08:57:43 -0500 (CDT), Mike Isely <isely at isely.net> wrote:
> > This patch series (3 of them) basically implement the same fixes as was 
> > previously done for the userspace driver back in 2008.  The fixes are 
> > not a direct port; I coded the changes and obviously tested again.  
> > This is also why there are 3 patches not 2; the third one is a fix for 
> > another problem uncovered while debugging the fixed mode change.
> 
> Mike, thanks for the patches!
> 
> I think before we proceed, the question I want to ask is whether it is
> preferable to add one super LVDS option (i.e. to parse a parameter string
> ala intelfb or video=) which we can use to program the panel configuration
> data (channels, bit depth, fixed mode, etc) or to add them ad hoc as
> individual parameters?

I'd actually prefer a single "super LVDS" option, but I think we need 
this as the first step.  We could build a little parser for that option, 
maybe even make possible direct specification of the LVDS fixed mode.

I did look into making possible direct specification of the fixed mode, 
but I'm really not fluent in the DRM "landscape" and after spending a 
day studying the DRM's video mode parser I felt it was less risky in the 
short term to just have a way to disable the scaling, same as what I had 
down for the user space driver back in 2008.

I'd be happy to help build a better overall solution here with a single 
"super LVDS" option.  But I don't want to step on any effort there that 
might already be underway.  In the mean time we needed *something* 
here...  And these patches really are pretty simple and low risk.

If the desire is to go to a single LVDS option to cover everything, then 
let's please get that implemented.  If not, then let's merge these 
patches that already exist and work.  What I don't want to see happen is 
nothing done at all...


> 
> One concern I have is the one Dave raised: we must be careful when
> allowing the user to override panel configuration. The ultimate question
> being do we trust the hardware configuration data more than the user
> cut'n'pasting from a random forum?
> -Chris

I agree one must be careful, but I think it's also very important that 
the user, with suitable warnings in the documentation (where would this 
be documented?), be able to override anything here.  If the decision is 
made to trust the hardware over the user, then I'm dead here because the 
i915 module right now is assuming that the video BIOS did things right, 
which is simply impossible when the LVDS display device is not integral 
to the processor board where the video BIOS resides.  That entire issue 
after all is what drove these patches in the first place.

IMHO, the user should always be able to override detected settings, 
especially when it's known that the detection can get things wrong.  
These fixes were merged into the userspace driver in 2008; things seemed 
to work out well there...

Being able to detect as much as possible is always a good thing.  But 
that's different than not providing any means of override.

  -Mike

-- 

Mike Isely
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8