RandR (etc) DriverFunc

[Andy Ritger]

On Wed, 16 Mar 2005, Egbert Eich wrote:

> Andy Ritger writes:
>  > 
>  > Sorry I didn't get a chance to respond to this yesterday, Thomas
>  > and Egbert.
>  > 
>  > This is a good point, Thomas.
>  > 
>  > In the SetConfig case, it's nice for the driver to be notified
>  > when the size is changed, even if the rotation is not changing (eg,
>  > to reallocate vidmem for the root window's new resolution), though
>  > I'd say it's not crucial (atleast for the NVIDIA driver).
> 
> Hm, when we change the size we call setMode() in the driver
> anyway. Doesn't this suffice?
> Currently we don't reallocate vidmem as we don't ever change the
> size of the video memory. This is not really supported in our
> model. This shortcoming needs to be resolved when reworking the
> driver.

Yes, the driver has all the information it needs wrt resizing.  Nevermind.

>  > 
>  > There does seem to be a fundamental problem with interpretting the
>  > return value of DriverFunc:
>  > 
>  >     TRUE  - DriverFunc() recognized xorgDriverFuncOp and successfully
>  >             handled the request
>  > 
>  >     FALSE - DriverFunc() recognized xorgDriverFuncOp but failed
>  >             while processing it, in which case the caller should
>  >             presumably abort its operation
> 
> Not necessarily. We may use the same fallbacks as below. If this
> isn't feasable we may want to consider to return a value in the 
> structure pointed to that informs us why we failed.

Good idea.

>  >                 - or -
>  > 
>  >             DriverFunc() did not recognize xorgDriverFuncOp; it is
>  >             caller-specific how to react to this.  In some cases,
>  >             the caller should probably continue, but in other
>  >             cases, we might want to bail out because DriverFunc()
>  >             doesn't provide the support we need.
>  > 
>  > There are 3 possible states that the common X server code needs to
>  > infer from the X driver's boolean return value :(
> 
> Yes, that's true. If you cannot handle 'not supported' and 'failed'
> the same way we either need to have a ternary return value or
> return a state in the private structure.
> 
>  > 
>  > At this point, there are drivers shipping (eg: the NVIDIA driver)
>  > that plug into DriverFunc to provide rotation support, so I'd ask
>  > that we not change the semantics of DriverFunc() or otherwise break
>  > binary compatibility.
> 
> Right.
> 
>  > 
>  > This proc was originally named RRFunc and was intended to be specific
>  > to RandR.  Egbert generalized it so that it could be used for other
>  > things, which seems like a good idea, except for this issue of
>  > overloading the return value.
> 
> See above. The return state is something private to the function.
> So it makes sense to put the return status (which may have many
> different states) into the private data structure passed back and
> forth.
> I think the solution we have right now will do what we need in a 
> reasonalbe way.

Yes, that seems good.

Thanks,
- Andy

>  > 
>  > I expect DriverFunc won't scale well in the future due to this
>  > return value problem.
>  > 
>  > I'll offer a solution; it may not be the best solution, but it does
>  > maintain binary compatibility and solves the return value dilemma
>  > going forward:
>  > 
>  >     - rename DriverFunc back to RRFunc and only use it for
>  >       RR_GET_INFO and RR_SET_CONFIG.  We alter the semantics of
>  >       this proc such that if a driver provides this function then
>  >       it must recognize both those values.
>  > 
>  >     - add a new proc to ScrnInfoRec named DriverFunc.  This new
>  >       DriverFunc would return a tri-state enum; eg:
>  > 
>  >         typedef enum {
>  >             DRIVER_FUNC_SUCCESS = 0,
>  >             DRIVER_FUNC_FAILURE = 1,
>  >             DRIVER_FUNC_UNRECOGIZED = 2
>  >         } DriverFuncReturnCodes;
>  > 
>  > I'm not sure how this solution would impact the
>  > GET_REQUIRED_HW_INTERFACES request used with the current DriverFunc.
>  > This solution would also break compile-time compatibility with
>  > drivers built outside the Xorg tree.  We might want to name the new
>  > proc "DriverFunc2" or something different so as not to be confused
>  > with the current DriverFunc.
>  > 
>  > Thoughts?
>  > 
> 
> Please see above. 
> 
> Cheers,
> 	Egbert.
>