<p>Thanks for writing it!</p>
<p>Reviewed-by: Corbin Simpson &lt;<a href="mailto:MostAwesomeDude@gmail.com">MostAwesomeDude@gmail.com</a>&gt;</p>
<p>Sending from a mobile, pardon my terseness. ~ C.</p>
<div class="gmail_quote">On Nov 6, 2011 10:39 AM, &quot;Tormod Volden&quot; &lt;<a href="mailto:lists.tormod@gmail.com">lists.tormod@gmail.com</a>&gt; wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Sun, Nov 6, 2011 at 4:37 PM, Corbin Simpson<br>
&lt;<a href="mailto:mostawesomedude@gmail.com">mostawesomedude@gmail.com</a>&gt; wrote:<br>
&gt; Trusting the spec worries me; could this break anybody?<br>
<br>
It will allow the user to override possibly safer default settings. If<br>
a user has specified, say, 2x in his xorg.conf and this was not<br>
honored because the card only reported 4x, this patch would finally<br>
give him the 2x he is asking for. If 2x turns out to not work, he will<br>
suddenly have problems. But the solution would be to fix his<br>
xorg.conf. Up till now he might be thinking 2x works fine, while he<br>
was actually getting another rate.<br>
<br>
Generally, the default rate is defined by the DDX and the patch does<br>
not change these policies. However, with the patch we may actually<br>
apply the default value, instead of an undefined &quot;0x&quot; (i.e. the case<br>
when the DDX sets 1x or 2x by default, but the card says only 4x). The<br>
spec (I have checked section 6.1.10) does not say anything about<br>
writing a zero rate, and I don&#39;t know what the silicon does. I don&#39;t<br>
know of any way to read out the resulting speed. Is there any simple<br>
agp benchmarking tools?<br>
<br>
Of course, it would be good if more people who currently see &quot;0x&quot; in<br>
dmesg would try this patch.<br>
<br>
Note that Dave Jones did the same for bridges in<br>
28af24bb8470c7d0573b703a2955548b73a6c066 to get rid of &quot;0x&quot; cases.<br>
<br>
&gt;<br>
&gt; Is there any reason to use the not-so-magic numbers instead of the named<br>
&gt; constants?<br>
<br>
I followed the style of the surrounding code. I can follow up with a<br>
patch using named constants everywhere if that would be desired.<br>
<br>
Thanks for looking at it!<br>
Tormod<br>
<br>
<br>
&gt;<br>
&gt; Sending from a mobile, pardon my terseness. ~ C.<br>
&gt;<br>
&gt; On Nov 6, 2011 7:03 AM, &quot;Tormod Volden&quot; &lt;<a href="mailto:lists.tormod@gmail.com">lists.tormod@gmail.com</a>&gt; wrote:<br>
&gt;&gt;<br>
&gt;&gt; From: Tormod Volden &lt;<a href="mailto:debian.tormod@gmail.com">debian.tormod@gmail.com</a>&gt;<br>
&gt;&gt;<br>
&gt;&gt; Some cards report that they support only 4x, in which case they<br>
&gt;&gt; should support 2x and 1x as well, according to the AGP spec.<br>
&gt;&gt;<br>
&gt;&gt; Otherwise a requested 1x or 2x rate will result in 0 being set:<br>
&gt;&gt;<br>
&gt;&gt; agpgart-via 0000:00:00.0: putting AGP V2 device into 0x mode<br>
&gt;&gt;<br>
&gt;&gt; For instance ProSavage KN133 [5333:8d02] only reports 4x.<br>
&gt;&gt;<br>
&gt;&gt; Signed-off-by: Tormod Volden &lt;<a href="mailto:debian.tormod@gmail.com">debian.tormod@gmail.com</a>&gt;<br>
&gt;&gt; ---<br>
&gt;&gt;  drivers/char/agp/generic.c |   18 ++++++++++++++++++<br>
&gt;&gt;  1 files changed, 18 insertions(+), 0 deletions(-)<br>
&gt;&gt;<br>
&gt;&gt; diff --git a/drivers/char/agp/generic.c b/drivers/char/agp/generic.c<br>
&gt;&gt; index b072648..c5d04e5 100644<br>
&gt;&gt; --- a/drivers/char/agp/generic.c<br>
&gt;&gt; +++ b/drivers/char/agp/generic.c<br>
&gt;&gt; @@ -526,6 +526,24 @@ static void agp_v2_parse_one(u32 *requested_mode, u32<br>
&gt;&gt; *bridge_agpstat, u32 *vga_<br>
&gt;&gt;                break;<br>
&gt;&gt;        }<br>
&gt;&gt;<br>
&gt;&gt; +       /* Some graphic cards report they only support 4x, however the AGP<br>
&gt;&gt; 2.0 spec<br>
&gt;&gt; +        * (section 4.1.1) says components must support the lower speeds<br>
&gt;&gt; as well.<br>
&gt;&gt; +        */<br>
&gt;&gt; +       switch (*vga_agpstat &amp; 7) {<br>
&gt;&gt; +       case 4:<br>
&gt;&gt; +               *vga_agpstat |= (AGPSTAT2_2X | AGPSTAT2_1X);<br>
&gt;&gt; +               printk(KERN_INFO PFX &quot;Graphics card claims to only support<br>
&gt;&gt; x4 rate. &quot;<br>
&gt;&gt; +                       &quot;Fixing up support for x2 &amp; x1\n&quot;);<br>
&gt;&gt; +               break;<br>
&gt;&gt; +       case 2:<br>
&gt;&gt; +               *vga_agpstat |= AGPSTAT2_1X;<br>
&gt;&gt; +               printk(KERN_INFO PFX &quot;Graphics card claims to only support<br>
&gt;&gt; x2 rate. &quot;<br>
&gt;&gt; +                       &quot;Fixing up support for x1\n&quot;);<br>
&gt;&gt; +               break;<br>
&gt;&gt; +       default:<br>
&gt;&gt; +               break;<br>
&gt;&gt; +       }<br>
&gt;&gt; +<br>
&gt;&gt;        /* Check the speed bits make sense. Only one should be set. */<br>
&gt;&gt;        tmp = *requested_mode &amp; 7;<br>
&gt;&gt;        switch (tmp) {<br>
&gt;&gt; --<br>
&gt;&gt; 1.7.5.4<br>
&gt;&gt;<br>
&gt;&gt; _______________________________________________<br>
&gt;&gt; dri-devel mailing list<br>
&gt;&gt; <a href="mailto:dri-devel@lists.freedesktop.org">dri-devel@lists.freedesktop.org</a><br>
&gt;&gt; <a href="http://lists.freedesktop.org/mailman/listinfo/dri-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/dri-devel</a><br>
&gt;<br>
</blockquote></div>