radeonfb(4) resolution

Hi all,

radeonfb(4) works fine using the EDID data, automatically
detecting
the resolution and videomode required for the monitor.
Nice!

However, I noticed a problem with the virtual
resolution/physical
resolution distinction.
The EDID picker will try to select the best physical
resolution based
on parameters such as "preffered mode" or
"best refreshing rate". But
the virtual resolution picker will always build a list of
all
resolutions and pick the largest one.

For example, these are the EDID video modes for my monitor:
        1024x768  89Hz
        1280x960  72Hz
>From the list, the physical resolution picker will
select the
"Preferred mode: 1024x768  89Hz". However, the
virtual resolution
picker will select the largest one, 1280x960  72Hz.

This means that not all the virtual screen will be visible
in the
physical screen.

The attached patch tries to deal with this as follows: for
each
monitor, instead of adding all video modes to the list, add
only the
best video mode (the preferred mode or the one with a
better
refreshing rate). Then, the largest resolution from the list
will be
choosed (as it was before).

I hope that this patch is useful.
Before the patch, the dmesg was:

[snip]
Preferred mode: 1024x768  89Hz
radeonfb0: display 0: initial virtual resolution 1280x960 at
32 bpp
radeonfb0: port 0: physical 1024x768 89Hz
radeonfb0: port 1: physical 1024x768 89Hz
[snip]

After the patch, it is:

[snip]
Preferred mode: 1024x768  89Hz
radeonfb0: display 0: initial virtual resolution 1024x768 at
32 bpp
radeonfb0: port 0: physical 1024x768 89Hz
radeonfb0: port 1: physical 1024x768 89Hz
[snip]

--
Thanks,
Marco.

Hi,

On 9/20/07, Michael Lorenz <macallannetbsd.org> wrote:
> > This means that not all the virtual screen will be
visible in the
> > physical screen.
>
> This may or may not have been intentional - after all,
radeonfb
> wasn't originally written with macppc in mind and I
don't know if the
> device it was written for actually uses the virtual
resolution. Looks
> like a bug to me though.

I think the intention was to select the largest resolution
of all
monitors in case there are multiple monitors. But in this
case each
monitor has more than one mode, and this confuses the
algorithm, which
will merge the two modes in the list instead of only the
actual used
mode for that monitor.

---
Thanks,
Marco

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hello,

On Sep 20, 2007, at 06:49, Marco Trillo wrote:

> On 9/20/07, Michael Lorenz <macallannetbsd.org> wrote:
>>> This means that not all the virtual screen will
be visible in the
>>> physical screen.
>>
>> This may or may not have been intentional - after
all, radeonfb
>> wasn't originally written with macppc in mind and I
don't know if the
>> device it was written for actually uses the virtual
resolution. Looks
>> like a bug to me though.
>
> I think the intention was to select the largest
resolution of all
> monitors in case there are multiple monitors. But in
this case each
> monitor has more than one mode, and this confuses the
algorithm, which
> will merge the two modes in the list instead of only
the actual used
> mode for that monitor.

Yeah, I'm just a bit reluctant to mess with this code since
I can't  
test it in the environment it was intended for and the
person who  
wrote it is out of reach for now. In the macppc case we'd
probably  
want to select modes for each output independently ( which
doesn't  
happen right now - if the firmware supplies an EDID block
both heads  
use it. I should fix that at some point  )

have fun
Michael
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)

iQEVAwUBRvKNccpnzkX8Yg2nAQJbEQf/VVi+V9nkI55kYsj0K4fYpKMEaboA
c775
d5fceW+gOIKGb/Gg0tmOKAPWF/Zcb0eHzwXI2pPPsR56grEJkfMCtCLMbOuk
vOX0
FGjzB2kUQfHem1AtTVHwgzn87ac4ENRe0NtbvfDqzr7rtDJK5X/TZqtFnx+H
qOJu
Q7Tr1C0C6RQmiIwUqh+yJEL8n1BeXTySp8BYpkwHcEIJ47jz7OqG26hiVd8G
Fhf+
5bAf/Y5BhFn72EwWVddQwqLCGIyMI/3RZvSA12KqNe+oZ+WmPcpGLSU1GNM4
Q4Gg
JfwXZJ+6djFR0HJX2LIpUue5rB2UiRMWJT3jSNUAm1k1t6//HxL/Cw==
=YdWm
-----END PGP SIGNATURE-----

Hi,

Based on your comments, I have a new patch that doesn't mess
with the
radeonfb resolution functions.

Instead, if static EDID modes are available for a port,
choose the
best mode from the list and use that as the only EDID mode
for that
port.

Also, use static EDID modes only if a port connector type
is
RADEON_CONN_CRT. For example on my eMac there are two ports,
one is
the internal monitor (CRT connector type) and the other port
has
another connector type (the external monitor port, I guess,
although I
have never used it). So it should now be possible to use
static EDID
for the internal monitor and a different EDID mode from the
external
monitor.

By the way, this patch also implements underline support for
the
radeonfb putchar function. Works fine for me. In fact, it
doesn't only
add underline support but also other attributes (such as
invert,
highlight, etc.) -- these are provided by the generic
rasops_allocattr() function (instead of using a
radeonfb_allocattr()
which didn't do anything special and which dind't care about
these
attributes).

The changes work fine for me, I hope that they're usfeul.
The radeonfb
dmesg messages with the patch applied:

radeonfb0 at pci0 dev 16 function 0: ATI Technologies Radeon
9200 5962
radeonfb0: Video BIOS not present
radeonfb0: No video BIOS, using default clocks
radeonfb0: refclk = 27.000 MHz, refdiv = 12 minpll = 125000,
maxpll = 350000
max_dotclock according to supported modes: 122240
radeonfb0: port #1: using static EDID
radeonfb0: 64 MB aperture at 0x98000000, 64 KB registers at
0x90000000
radeonfb0: display 0: initial virtual resolution 1024x768 at
32 bpp
radeonfb0: port 0: physical 1024x768 60Hz
radeonfb0: port 1: physical 1024x768 89Hz
init engine
wsdisplay0 at radeonfb0 kbdmux 1: console (fb, vt100
emulation)

In this case, the internal monitor is at port 1, which uses
the static EDID.

     -Marco

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hello,

On Sep 27, 2007, at 09:00, Marco Trillo wrote:

> Also, use static EDID modes only if a port connector
type is
> RADEON_CONN_CRT. For example on my eMac there are two
ports, one is
> the internal monitor (CRT connector type) and the other
port has
> another connector type (the external monitor port, I
guess, although I
> have never used it). So it should now be possible to
use static EDID
> for the internal monitor and a different EDID mode from
the external
> monitor.

I think this is problematic. Think of all those laptops with
a TFT as  
primary monitor, with that change they wouldn't be able to
use static  
EDID and not all panels provide DDC2 data ( the one in my
iBook G4  
does but that's apparently an exception )
The proper solution would be me getting around and hack the
kernel  
pass an indication which port the static EDID block has been
read  
from, then radeonfb could do The Right Thing.

> By the way, this patch also implements underline
support for the
> radeonfb putchar function. Works fine for me. In fact,
it doesn't only
> add underline support but also other attributes (such
as invert,
> highlight, etc.) -- these are provided by the generic
> rasops_allocattr() function (instead of using a
radeonfb_allocattr()
> which didn't do anything special and which dind't care
about these
> attributes).

Yeah, attribute support has been lacking in all fb drivers,
inverse  
being the only one that (usually) worked. Maybe it can be
abstracted  
out since it would likely do the same thing on all drivers?

Thanks for your work, I hope I'll have time to have a closer
look at  
the patch later today.

have fun
Michael
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)

iQEVAwUBRvvj6cpnzkX8Yg2nAQJviAf6Al9yjp744/zheU7V0DftsWHICIO9
D4e7
JpFgwJ/7t4ZU0LxSmLQx/CU7Xz5pUe0Zf1LpMsYX7IkE2lmgTaDYp8XudRAD
C3ME
lHWOJSOJqRIh4Ou/euEXyKbJwgIyfeuVJhSpmhGts+P1JPpimF5tR9biPP8J
S953
PvlILEwDute16Hn8x9kvY+s5hJ01Hwt1coZHpaWbJ4RawsAbpXyI8swzUene
9X3l
PrWbL0vp967m9V3nTZpzcSFyuNCUyqFZCgwaf2zLmycwhwXCJU3tHxRm1qFG
aHzY
r6bWO5z+xQtB5K5AssydNsM8aLQ4Bn7AR5MsRVOUCKdDfr6RgSwWOw==
=p1WM
-----END PGP SIGNATURE-----