[Intel-gfx] [PATCH 0/3] [drm/i915] - LVDS mode setting fixes
isely at isely.net
Thu Mar 17 07:56:17 PDT 2011
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?
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.
isely @ isely (dot) net
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
More information about the Intel-gfx