[PATCH 07/23] staging: xgifb: avoid direct references xgi_video_info

Aaro Koskinen aaro.koskinen at iki.fi
Sun Oct 9 21:31:21 UTC 2011


Hi,

On Sun, 9 Oct 2011, Dan Carpenter wrote:
> On Sun, Oct 09, 2011 at 11:22:07PM +0300, Aaro Koskinen wrote:
>> On Sun, 9 Oct 2011, Dan Carpenter wrote:
>>> On Sun, Oct 09, 2011 at 07:42:51PM +0300, Aaro Koskinen wrote:
>>>> -	switch (xgi_video_info.disp_state & DISPTYPE_DISP2) {
>>>> +	switch (xgifb_info->disp_state & DISPTYPE_DISP2) {
>>>
>>> This isn't related to this patch, but this data type is weird.
>>>
>>> It would be probably be more natural to break up ->disp_state into
>>> two variables where one stored the display type and one stored the if
>>> the displays were mirrored or single.  Can you connect a CRT and an
>>> LCD at the same time?
>>
>> Based on a quick look, the driver does not even use the single/mirror
>> info for anything...
>>
>> It should be possible to connect CRT+LCD at the same time.
>
> So then how does this switch statement work if they're both set?  It
> reverts to the default case.  That's the oddness of using a bitfield
> in a switch statement.
>
> But again, this patch is fine as is, I was just wondering about that
> unrelated thing.

Sorry, I misunderstood the question. DISPTYPE_DISP2 is used to get the
type of the second output/port. It can be CRT2, LCD, or TV but only one
of them is set at a time, so the code should work. Then there is also
the first display port, which is always hardcoded to CRT1 (the external
VGA output). So CRT+LCD is possible but not in the same output.

A.



More information about the devel mailing list