Re: Unblock xserver-xorg-video-geode
On Sat, 18 Sep 2010 18:36:29 +0300
Martin-Éric Racine <q-funk@iki.fi> wrote:
> 2010/9/3 Martin-Éric Racine <q-funk@iki.fi>:
> > On Fri, Sep 3, 2010 at 11:46 AM, Julien Cristau
> > <jcristau@debian.org> wrote:
> >> On Fri, Sep 3, 2010 at 09:33:19 +0800, Huang, FrankR wrote:
> >>
> >>> I saw this thread from top to the end.
> >>> Is it because the patch
> >>> http://cgit.freedesktop.org/xorg/driver/xf86-video-geode/commit/?id=e9447f5335681a78cf87ebf8c9659a6fecfc9746
> >>> block the geode driver to be accepted?
> >>> If that is so, can we agree with the suggestion from Julien?
> >>> if ((pGeode->Output & (OUTPUT_PANEL | OUTPUT_DCON))
> >>> == (OUTPUT_PANEL | OUTPUT_DCON))
> >>>
> >> Well, or you could make it if (pGeode->Output == (OUTPUT_PANEL |
> >> OUTPUT_DCON)). Or alternatively, explain why the existing code is
> >> correct.
> >
> > We'll discuss this particular one with Otavio and let him comment on
> > the solution.
>
> Otavio, we're still waiting for your explanation on this issue.
>
> Julien, is there any other issue left to address, before this unblock
> request can be approved?
>
> Best Regards,
> Martin-Éric
>
>
This is pretty clearly a bug. This should fix it (after all, that
check is merely to see if the panel is a DCON; we don't care at
all about the panel bit). This also adds an extra paren in the
following if() statement for clarity.
I'm resisting the temptation to change GeodeRec's Output member
to an unsigned long (for now). Bitfields should really be
unsigned.
Signed-off-by: Andres Salomon <dilinger@queued.net>
--- a/src/lx_output.c 2010-09-20 18:03:52.000000000 +0000
+++ b/src/lx_output.c 2010-09-20 18:23:41.000000000 +0000
@@ -156,13 +156,13 @@
GeodeRec *pGeode = GEODEPTR(pScrni);
/* DCON Panel specific resolution - OLPC's one */
- if (pGeode->Output & (OUTPUT_PANEL | OUTPUT_DCON)) {
+ if (pGeode->Output & OUTPUT_DCON) {
if (pGeode->panelMode->HDisplay == 1200 &&
pGeode->panelMode->VDisplay == 900)
return MODE_OK;
}
- if (pGeode->Output & OUTPUT_PANEL &&
+ if ((pGeode->Output & OUTPUT_PANEL) &&
gfx_is_panel_mode_supported(pGeode->panelMode->HDisplay,
pGeode->panelMode->VDisplay,
pMode->HDisplay,
Reply to: