[PATCH modesetting] Implement ->driverFunc

Adam Jackson ajax at redhat.com
Fri Jul 20 15:59:08 PDT 2012


On 7/20/12 6:37 PM, Aaron Plattner wrote:
> On 07/20/2012 03:28 PM, Adam Jackson wrote:
>> On 7/20/12 3:44 PM, Aaron Plattner wrote:
>>> On 07/19/2012 03:15 PM, Adam Jackson wrote:
>>>> +static Bool
>>>> +ms_driver_func(ScrnInfoPtr scrn, xorgDriverFuncOp op, void *data)
>>>> +{
>>>> +    xorgHWFlags *flag;
>>>> +
>>>> +    switch (op) {
>>>> +    case GET_REQUIRED_HW_INTERFACES:
>>>> +        flag = (CARD32 *)data;
>>>> +        (*flag) = 0;
>>>
>>> Should this be = 0, or &= ~HW_IO?
>>
>> It should not be ~HW_IO, since that would set HW_SKIP_CONSOLE, which we
>> don't want.
>
> &=, not =.  I.e.,
>
>   *flag = *flag & ~HW_IO.
>
> I.e. just clear the HW_IO bit and leave the rest alone, rather than
> clear all of them.

Oh, sorry, misread you, but still no.  The call to driverFunc in 
InitOutput does not initialize the outparam before handing it to the 
driver.  (I'm not defending the interface, just describing what it 
happens to have always been.)  You'd have to clear anything you didn't 
want.  Which means...

> Just setting it to 0 will also clear any future on-by-default flags that
> we might want to add.

... this would become awkward since you couldn't write a future-proofed 
driver.

I think it makes more sense to treat the flags argument as 'reserved 
bits MBZ', although the server ought to zero the outparam first. 
HW_SKIP_CONSOLE even counts as precedent since that wasn't there initially.

- ajax


More information about the xorg-devel mailing list