Re: [PATCH] fb/intelfb: Do not depend on EMBEDDED

Re: [PATCH] fb/intelfb: Do not depend on EMBEDDED

[Jesse Barnes]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 2413 bytes --]

Right, the logic is that the driver really is for embedded (i.e. very special purpose) use.  It should not be selected unless you really know what you're doing or are building a very particular product.
If you can think of a better way of preventing users and distros from accidentally selecting this, then please send a patch.
Jesse
Dave Airlie <airlied@linux.ie> wrote:
>>> I am worried that the intelfb driver depends on EMBEDDED. I consider>> this an abuse of the EMBEDDED configuration option, which as I>> understand it was originally meant to expose fine-tuning options,>> rather than to arbitrarily disable drivers when not selected.>>Since we merged a kms driver for Intel hw that supports all intel chipsets>and more importantly all the outputs on Intel chipsets, intelfb should>be considered legacy at the least and broken on > 50% of intel hw.>>We left it in in that most ppl who wanted it were using it in embedded >configs, whereas for most users it just doesn't work, like I don't thinkit>supports LVDS which means loading it on a laptop will trash it.>>Dave>>>> >> So I suggest that we drop this dependency now.>> >> Signed-off-by: Jean Delvare <jdelvare@suse.de>>> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>>> Cc: Dave Airlie <airlied@linux.ie>>> --->> Jesse, in the original commit, you wrote that intelfb was "really a>> special purpose embedded driver". It looks like a perfectly standard>> framebuffer driver to me, which means that it may have users beyond>> embedded. For example I always prefer framebuffer over X for my>> servers. Or am I missing something and intelfb is really special?>> >>  drivers/video/Kconfig |    2 +->>  1 file changed, 1 insertion(+), 1 deletion(-)>> >> --- linux-2.6.32.orig/drivers/video/Kconfig	2009-12-03 08:48:34.000000000 +0100>> +++ linux-2.6.32/drivers/video/Kconfig	2009-12-11 10:57:43.000000000 +0100>> @@ -1121,7 +1121,7 @@ config FB_CARILLO_RANCH>>  >>  config FB_INTEL>>  	tristate "Intel 830M/845G/852GM/855GM/865G/915G/945G/945GM/965G/965GM support (EXPERIMENTAL)">> -	depends on EXPERIMENTAL && FB && PCI && X86 && AGP_INTEL && EMBEDDED>> +	depends on EXPERIMENTAL && FB && PCI && X86 && AGP_INTEL>>  	select FB_MODE_HELPERS>>  	select FB_CFB_FILLRECT>>  	select FB_CFB_COPYAREA>> >> >ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH] fb/intelfb: Do not depend on EMBEDDED

[Jean Delvare]

Hi Dave, Jesse,

Dave Airlie <airlied@linux.ie> wrote:
>> I am worried that the intelfb driver depends on EMBEDDED. I consider
>> this an abuse of the EMBEDDED configuration option, which as I
>> understand it was originally meant to expose fine-tuning options,
>> rather than to arbitrarily disable drivers when not selected.
>
>Since we merged a kms driver for Intel hw that supports all intel chipsets
>and more importantly all the outputs on Intel chipsets, intelfb should
>be considered legacy at the least and broken on > 50% of intel hw.

Are you suggesting that the "kms driver" is a drop-in replacement for
intelfb? More specifically, can I use that "kms driver" to get a
high-resolution console?

>We left it in in that most ppl who wanted it were using it in embedded 
>configs, whereas for most users it just doesn't work, like I don't think
>it supports LVDS which means loading it on a laptop will trash it.

The intelfb driver is marked EXPERIMENTAL, so it is already expected to not
work perfectly in all cases.

Le samedi 12 décembre 2009 22:55, Jesse Barnes a écrit :
> Right, the logic is that the driver really is for embedded (i.e. very
> special purpose) use.  It should not be selected unless you really
> know what you're doing or are building a very particular product.   

The Kconfig help text doesn't say anything about this.

My understanding is that the intelfb driver was not _designed_ to be
useful on embedded designs only. It just happens to be incomplete in
such a way that it works only in a few selected cases, which happen
to be embedded cases, and it fails in many other cases.

The proper way to handle this is not to make the driver depend on
EMBEDDED. The proper way would be to change the intelfb driver so
that it no longer binds to devices it will not properly support. If
the driver doesn't support LVDS (whatever it is) then it should
cleanly fail on systems which have that.

> If you can think of a better way of preventing users and distros
> from accidentally selecting this,

The reason why I sent a patch in the first place is exactly opposite:
I want to let distros select this driver. My case is as follows: we
had a product which included the intelfb driver, which we are in the
process of upgrading. Now we find that the intelfb driver is gone
(no longer selectable), which causes a problem as far as the upgrade
path of our customers is concerned.

So the problem I have to solve is: given a customer who was
successfully using the intelfb driver before, what solutions can we
offer when said customer upgrades to our new product? My own solution
was straightforward: keep including the intelfb driver in the new
product. Thus my patch dropping the dependency on EMBEDDED. If
another solution exists, please let me know.

> then please send a patch. 

First of all, I'd need to better understand how the various drivers
relate to each other, and what functionality they provide. In my
little outdated mind, framebuffer is for high-resolution console
without X, while drm is for accelerated X. Apparently this changed
more or less recently, but the documentation wasn't updated.

It would help if the DRM option description was updated. It still
reads: "Direct Rendering Manager (XFree86 4.1.0 and higher DRI
support)". If the DRM core is now also providing support for
framebuffer-like functionality (again, if I understand correctly)
then the reference to XFree86 should be dropped. The help text
should also be updated to properly describe all that the DRM core
offers today.

Honestly, I'm probably not the best person to write this text, as I
don't know much about the current state of this graphics stuff.

Thanks,
-- 
Jean Delvare
Suse L3

Re: [PATCH] fb/intelfb: Do not depend on EMBEDDED

[Dave Airlie]

> >
> >Since we merged a kms driver for Intel hw that supports all intel chipsets
> >and more importantly all the outputs on Intel chipsets, intelfb should
> >be considered legacy at the least and broken on > 50% of intel hw.
> 
> Are you suggesting that the "kms driver" is a drop-in replacement for
> intelfb? More specifically, can I use that "kms driver" to get a
> high-resolution console?

Yes, thats all it does, but you need to make suer the X.org userspace you 
ship supports which for Intel it has done for a while.

> My understanding is that the intelfb driver was not _designed_ to be
> useful on embedded designs only. It just happens to be incomplete in
> such a way that it works only in a few selected cases, which happen
> to be embedded cases, and it fails in many other cases.

Well I wrote a large chunk of it and it was for an embedded system.

> The proper way to handle this is not to make the driver depend on
> EMBEDDED. The proper way would be to change the intelfb driver so
> that it no longer binds to devices it will not properly support. If
> the driver doesn't support LVDS (whatever it is) then it should
> cleanly fail on systems which have that.

LVDS are laptops, adding code to fix intelfb to know when its failing
is pointless, kernel modesetting supports all the hw properly that intelfb
fails on.

> > If you can think of a better way of preventing users and distros
> > from accidentally selecting this,
> 
> The reason why I sent a patch in the first place is exactly opposite:
> I want to let distros select this driver. My case is as follows: we
> had a product which included the intelfb driver, which we are in the
> process of upgrading. Now we find that the intelfb driver is gone
> (no longer selectable), which causes a problem as far as the upgrade
> path of our customers is concerned.

We don't want distros using this driver going forward or shipping it,
any distro that has a reason to enable it needs to enable EMBEDDED,

We cannot have two drivers for this hardware enabled by default, and
having two options in the menus confuses users and distros alike, so
EMBEDDED is a clear sign. I'm tempted to make it CONFIG_BROKEN.

> So the problem I have to solve is: given a customer who was
> successfully using the intelfb driver before, what solutions can we
> offer when said customer upgrades to our new product? My own solution
> was straightforward: keep including the intelfb driver in the new
> product. Thus my patch dropping the dependency on EMBEDDED. If
> another solution exists, please let me know.

You can do that, just enable CONFIG_EMBEDDED, but that is a distro choice,
upsteram this driver is dead, you should see if the KMS solution is 
suitable for you customer and migrate them to it.

> First of all, I'd need to better understand how the various drivers
> relate to each other, and what functionality they provide. In my
> little outdated mind, framebuffer is for high-resolution console
> without X, while drm is for accelerated X. Apparently this changed
> more or less recently, but the documentation wasn't updated.
> 
> It would help if the DRM option description was updated. It still
> reads: "Direct Rendering Manager (XFree86 4.1.0 and higher DRI
> support)". If the DRM core is now also providing support for
> framebuffer-like functionality (again, if I understand correctly)
> then the reference to XFree86 should be dropped. The help text
> should also be updated to properly describe all that the DRM core
> offers today.

We should probably fix that alright, I'll have to think about how
we can do that.

Dave.

Re: [PATCH] fb/intelfb: Do not depend on EMBEDDED

[Jean Delvare]

Hi Dave,

Le dimanche 13 décembre 2009 22:53, Dave Airlie a écrit :
> > Are you suggesting that the "kms driver" is a drop-in replacement for
> > intelfb? More specifically, can I use that "kms driver" to get a
> > high-resolution console?
> 
> Yes, thats all it does, but you need to make sure the X.org userspace you 
> ship supports which for Intel it has done for a while.

... if and only if I need X.org at all, right? I sure hope that
installing X.org hasn't become mandatory just to have a
high-resolution console?

> > (...)
> > The proper way to handle this is not to make the driver depend on
> > EMBEDDED. The proper way would be to change the intelfb driver so
> > that it no longer binds to devices it will not properly support. If
> > the driver doesn't support LVDS (whatever it is) then it should
> > cleanly fail on systems which have that.
> 
> LVDS are laptops, adding code to fix intelfb to know when its failing
> is pointless,

I don't think it is that pointless. Failing gracefully, preferably
with a useful error message, is always important.

> kernel modesetting supports all the hw properly that intelfb fails on.

True but irrelevant. If I load a driver and it crashes my system,
that's bad, even if there is another driver available that would
not have crashed my system (and may even have made something useful
out of it.) Again, we want drivers to fail gracefully, and point the
user to alternatives if applicable.

> (...)
> We don't want distros using this driver going forward or shipping it,
> any distro that has a reason to enable it needs to enable EMBEDDED,
> 
> We cannot have two drivers for this hardware enabled by default, and

Actually we sort of can, that's the power of modules. I'm not sure
what "make allyesconfig" would do though... does it honnor
"default n" in Kconfig?

> having two options in the menus confuses users and distros alike, so
> EMBEDDED is a clear sign. I'm tempted to make it CONFIG_BROKEN.

The remaining users of the intelfb driver may disagree, but I'd
indeed prefer the driver to be marked as broken, if that's what it
is. Or even removed from the kernel tree. If they want to keep it
in the tree, they should fix it.

> > So the problem I have to solve is: given a customer who was
> > successfully using the intelfb driver before, what solutions can we
> > offer when said customer upgrades to our new product? My own solution
> > was straightforward: keep including the intelfb driver in the new
> > product. Thus my patch dropping the dependency on EMBEDDED. If
> > another solution exists, please let me know.
> 
> You can do that, just enable CONFIG_EMBEDDED, but that is a distro choice,

Enabling CONFIG_EMBEDDED would give us access to other tweaking
options that we don't want to touch, and I have no idea if any of
these is set by default. So thanks but no thanks. I'm much better
just dropping the dependency on EMBEDDED for intelfb, at least I
control the scope of this change.

> upstream this driver is dead, you should see if the KMS solution is 
> suitable for you customer and migrate them to it.

Except that I have no clue if any of my customers are actually
using it, nor who they are, nor what hardware they use. As it stands,
I would know if we were to drop intelfb in our next product, because
these customers would complain, probably loudly. But obviously this
is no solution business-wise.

If I wanted to attempt a transparent migration, where would I start?
As I recall, console on framebuffer needs specific boot parameters,
right? I don't suppose that kms will transparently pick them and do
the right thing, do I? If there documentation available on how one
can get a high resolution console using kms?

Thanks,
-- 
Jean Delvare
Suse L3

Re: [PATCH] fb/intelfb: Do not depend on EMBEDDED

[Jesse Barnes]

On Wed, 16 Dec 2009 14:37:31 +0100
Jean Delvare <jdelvare@suse.de> wrote:
> If I wanted to attempt a transparent migration, where would I start?
> As I recall, console on framebuffer needs specific boot parameters,
> right? I don't suppose that kms will transparently pick them and do
> the right thing, do I? If there documentation available on how one
> can get a high resolution console using kms?

Actually, that's one of the big advantages of the KMS based driver.  It
*does* transparently pick the native resolution of the detected
output(s) and set up a high res console.  You can provide boot options
to override it, but unlike intelfb, boot options aren't necessary to
get things working in the first place.

If you want to run X later, you'll need an updated xf86-video-intel
(preferably 2.9.x) or xf86-video-radeon driver as well, since code to
support the KMS interfaces landed only slightly before 2.6.29 came out
in the case of Intel.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] fb/intelfb: Do not depend on EMBEDDED

[Krzysztof Halasa]

Jesse Barnes <jbarnes@virtuousgeek.org> writes:

> Actually, that's one of the big advantages of the KMS based driver.  It
> *does* transparently pick the native resolution of the detected
> output(s) and set up a high res console.  You can provide boot options
> to override it, but unlike intelfb, boot options aren't necessary to
> get things working in the first place.

How do I set the (default) resolution and timings with non-EDID display
(e.g. analog RGB TV)?

fbset seems to only set the "logical" resolution, doesn't like the
pixclock.
-- 
Krzysztof Halasa

Re: [PATCH] fb/intelfb: Do not depend on EMBEDDED

[Dave Airlie]

> 
> > Actually, that's one of the big advantages of the KMS based driver.  It
> > *does* transparently pick the native resolution of the detected
> > output(s) and set up a high res console.  You can provide boot options
> > to override it, but unlike intelfb, boot options aren't necessary to
> > get things working in the first place.
> 
> How do I set the (default) resolution and timings with non-EDID display
> (e.g. analog RGB TV)?
> 
> fbset seems to only set the "logical" resolution, doesn't like the
> pixclock.

Currently with the command line video=, this isn't optimal but still
nobody has come up with a way to make fbset useful since it can't deal
with multiple heads.

also the old intelfb didn't deal with analog rgb tv at all.

Dave.

Re: [PATCH] fb/intelfb: Do not depend on EMBEDDED

[Krzysztof Halasa]

Dave Airlie <airlied@linux.ie> writes:

> Currently with the command line video=, this isn't optimal but still
> nobody has come up with a way to make fbset useful since it can't deal
> with multiple heads.

Ahh, now I can see it.

> also the old intelfb didn't deal with analog rgb tv at all.

Well... actually, it did: intelfb works very well if a correct mode is
set (a trivial patch adjusting the clock lower limit to 13.5 MHz may be
needed, or one can use the double clock and resolution, or maybe 1:1
pixel aspect ratio etc). It's not about the CH7xxx encoder, the TV is
connected directly to VGA output.

E.g.

# PAL 720x576, 50 Hz, Interlaced (13.5 MHz dotclock)
#
#                               Horizontal      Vertical
#       Resolution              720             576
#       Scan Frequency          15.625 kHz      50 Hz (I)
#       Sync Width              4.296 us        1.280 ms
#       Front Porch             1.000 us        0.256 ms
#       Back Porch              5.370 us        1.600 ms
#       Active Time             53.333 us       36.864 ms
#       Blank Time              10.667 us       3.136 ms
#       Polarity                Negative        Negative
#
#       H:      720 + 14 +  58 + 72 = 864
#       V:      576 +  4 +  20 + 25 = 625

mode "PAL"
        # D: 13.5 MHz, H: 15.625 kHz, V: 50 Hz
        geometry 720 576 720 576 32
        timings 74074 72 14 25 4 58 20
        laced true
        hsync low
        vsync low
endmode

"fbset PAL" is all one needs to use it.

(I know i915 doesn't have interlaced mode support for analog VGA out,
not a problem).
-- 
Krzysztof Halasa

Re: [PATCH] fb/intelfb: Do not depend on EMBEDDED

[Jesse Barnes]

On Sun, 13 Dec 2009 12:50:13 +0100
Jean Delvare <jdelvare@suse.de> wrote:
> Le samedi 12 décembre 2009 22:55, Jesse Barnes a écrit :
> > Right, the logic is that the driver really is for embedded (i.e.
> > very special purpose) use.  It should not be selected unless you
> > really know what you're doing or are building a very particular
> > product.   
> 
> The Kconfig help text doesn't say anything about this.
> 
> My understanding is that the intelfb driver was not _designed_ to be
> useful on embedded designs only. It just happens to be incomplete in
> such a way that it works only in a few selected cases, which happen
> to be embedded cases, and it fails in many other cases.
> 
> The proper way to handle this is not to make the driver depend on
> EMBEDDED. The proper way would be to change the intelfb driver so
> that it no longer binds to devices it will not properly support. If
> the driver doesn't support LVDS (whatever it is) then it should
> cleanly fail on systems which have that.

Sorry my last message came across as a bit curt (I was writing on a
phone and didn't want to type anymore :).

Making intelfb not bind to unsupported devices would require a good
chunk of work; it would need to scan available outputs among other
things.

> The reason why I sent a patch in the first place is exactly opposite:
> I want to let distros select this driver. My case is as follows: we
> had a product which included the intelfb driver, which we are in the
> process of upgrading. Now we find that the intelfb driver is gone
> (no longer selectable), which causes a problem as far as the upgrade
> path of our customers is concerned.

Hopefully you can migrate your product to the KMS based fb driver
instead.  It should provide the same functionality but with a better
feature set (e.g. suspend/resume support).

> So the problem I have to solve is: given a customer who was
> successfully using the intelfb driver before, what solutions can we
> offer when said customer upgrades to our new product? My own solution
> was straightforward: keep including the intelfb driver in the new
> product. Thus my patch dropping the dependency on EMBEDDED. If
> another solution exists, please let me know.

Enabling EMBEDDED for your distro would be another option; if you have
a very specific product in mind and want to preserve the same features
and bugs, then that might be the best route in this particular case.

> It would help if the DRM option description was updated. It still
> reads: "Direct Rendering Manager (XFree86 4.1.0 and higher DRI
> support)". If the DRM core is now also providing support for
> framebuffer-like functionality (again, if I understand correctly)
> then the reference to XFree86 should be dropped. The help text
> should also be updated to properly describe all that the DRM core
> offers today.

Yeah, we do need better help text.  People still get confused about
i830 and i810 as well...

-- 
Jesse Barnes, Intel Open Source Technology Center

[PATCH] fb/intelfb: Do not depend on EMBEDDED

[Jean Delvare]

I am worried that the intelfb driver depends on EMBEDDED. I consider
this an abuse of the EMBEDDED configuration option, which as I
understand it was originally meant to expose fine-tuning options,
rather than to arbitrarily disable drivers when not selected.

So I suggest that we drop this dependency now.

Signed-off-by: Jean Delvare <jdelvare@suse.de>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Dave Airlie <airlied@linux.ie>
---
Jesse, in the original commit, you wrote that intelfb was "really a
special purpose embedded driver". It looks like a perfectly standard
framebuffer driver to me, which means that it may have users beyond
embedded. For example I always prefer framebuffer over X for my
servers. Or am I missing something and intelfb is really special?

 drivers/video/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- linux-2.6.32.orig/drivers/video/Kconfig	2009-12-03 08:48:34.000000000 +0100
+++ linux-2.6.32/drivers/video/Kconfig	2009-12-11 10:57:43.000000000 +0100
@@ -1121,7 +1121,7 @@ config FB_CARILLO_RANCH
 
 config FB_INTEL
 	tristate "Intel 830M/845G/852GM/855GM/865G/915G/945G/945GM/965G/965GM support (EXPERIMENTAL)"
-	depends on EXPERIMENTAL && FB && PCI && X86 && AGP_INTEL && EMBEDDED
+	depends on EXPERIMENTAL && FB && PCI && X86 && AGP_INTEL
 	select FB_MODE_HELPERS
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA

-- 
Jean Delvare
Suse L3

Re: [PATCH] fb/intelfb: Do not depend on EMBEDDED

[Dave Airlie]

> I am worried that the intelfb driver depends on EMBEDDED. I consider
> this an abuse of the EMBEDDED configuration option, which as I
> understand it was originally meant to expose fine-tuning options,
> rather than to arbitrarily disable drivers when not selected.

Since we merged a kms driver for Intel hw that supports all intel chipsets
and more importantly all the outputs on Intel chipsets, intelfb should
be considered legacy at the least and broken on > 50% of intel hw.

We left it in in that most ppl who wanted it were using it in embedded 
configs, whereas for most users it just doesn't work, like I don't thinkit
supports LVDS which means loading it on a laptop will trash it.

Dave


> 
> So I suggest that we drop this dependency now.
> 
> Signed-off-by: Jean Delvare <jdelvare@suse.de>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Cc: Dave Airlie <airlied@linux.ie>
> ---
> Jesse, in the original commit, you wrote that intelfb was "really a
> special purpose embedded driver". It looks like a perfectly standard
> framebuffer driver to me, which means that it may have users beyond
> embedded. For example I always prefer framebuffer over X for my
> servers. Or am I missing something and intelfb is really special?
> 
>  drivers/video/Kconfig |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- linux-2.6.32.orig/drivers/video/Kconfig	2009-12-03 08:48:34.000000000 +0100
> +++ linux-2.6.32/drivers/video/Kconfig	2009-12-11 10:57:43.000000000 +0100
> @@ -1121,7 +1121,7 @@ config FB_CARILLO_RANCH
>  
>  config FB_INTEL
>  	tristate "Intel 830M/845G/852GM/855GM/865G/915G/945G/945GM/965G/965GM support (EXPERIMENTAL)"
> -	depends on EXPERIMENTAL && FB && PCI && X86 && AGP_INTEL && EMBEDDED
> +	depends on EXPERIMENTAL && FB && PCI && X86 && AGP_INTEL
>  	select FB_MODE_HELPERS
>  	select FB_CFB_FILLRECT
>  	select FB_CFB_COPYAREA
> 
>