[Xorg-driver-geode] xf86-video-geode: Changes to 'master'

[Matt Turner]

On Mon, Oct 11, 2010 at 9:38 PM, Huang, FrankR <FrankR.Huang at amd.com> wrote:
> [snip]

OK, let me see if I can explain:

The purpose of the *_mode_valid function in each device-dependent X
(xf86-video-ati, xf86-video-intel) is to allow the driver to decide
whether a particular mode (mode is a resolution+color depth+refresh
rate) is acceptable for the hardware. So, it allows the driver to ask
"can I set a mode of 1024x768-32 at 60Hz?" and *_mode_valid either says
yes (MODE_OK) or no (MODE_BAD,MODE_BANDWIDTH,MODE_CLOCK,etc.). It, at
least in the case of xf86-video-geode's lx_output_mode_valid function,
does not modify either of its two arguments, but rather tests them and
returns whether the proposed mode is acceptable for hardware or not.

xf86-video-ati's has many checks: for bandwidth, for clock range, and
panel scaling. Clearly you can see that this code is in place to make
sure that the driver does not attempt to set a mode the hardware
cannot handle.

Otavio Salvador's May 29th commit adds additional requirements for a
mode to be considered valid. Apparently these additional checks are
too restrictive, and hence you made your change on Sept 29? I really
have no idea what you're trying to fix.

So, with your current code, regardless of input given to
lx_output_mode_valid, it always tells the driver that "this mode is
OK." This is wrong. For instance, I can ask lx_output_mode_valid
whether the totally bogus mode of 0x0-64 at 900Hz is acceptable and it
will gladly tell me MODE_OK (but still do a lot of work to determine
this result). Under no circumstances does lx_output_mode_valid ever
tell the driver that the proposed mode is bad. Clearly you can see
that this is wrong.

Because of this, Julien has (sarcastically) suggested that you change
lx_output_mode_valid to only return MODE_OK, which is totally
equivalent to the current code, since lx_output_mode_valid has no
side-effects and modifies none of its arguments. This is not a real
suggestion, as it's just as wrong as the current code.

So, what you need to do is explain what was wrong with the
lx_output_mode_valid function before you made the change. That is,
what proposed mode was incorrectly rejected? From here, I'm sure
someone can help you to find the appropriate fix.

Please search for 'mode_valid' in the xserver's
hw/xfree86/modes/xf86Crtc.c and hw/xfree86/modes/xf86RandR12.c to see
how xf86-video-geode's lx_output_mode_valid function is called.

Do you understand and agree with this (and Matthew and Adam's)
assessment? If not, please precisely explain why not.

Best of luck,
Matt