[PATCH] int10/vbe: don't use xf86Screens. (ABI) (v2)

[Dave Airlie]

On Wed, May 16, 2012 at 10:24 AM, Mark Kettenis <mark.kettenis at xs4all.nl> wrote:
>> Date: Wed, 16 May 2012 10:16:24 +0100
>> From: Dave Airlie <airlied at gmail.com>
>>
>> >
>> >> @@ -340,7 +341,7 @@ vbeDoEDID(vbeInfoPtr pVbe, pointer pDDCModule)
>> >>      if (!DDC_data)
>> >>          return NULL;
>> >>
>> >> -    pMonitor = xf86InterpretEDID(pVbe->pInt10->scrnIndex, DDC_data);
>> >> +    pMonitor = xf86InterpretEDID(pVbe->pInt10->pScrn->scrnIndex,
>> >> DDC_data);
>> >
>> >
>> > The callee here wants index->ptr conversion too, doesn't it?  I don't think
>> > I see that in subsequent patches.
>>
>> That gets into an area I've been thinking about but mostly avoiding for now,
>>
>> That is pretty much a logging function, i.e. scrnIndex only goes into
>> logging functions,
>>
>> Now I'm tempted to leave logging functions just passing indices, but
>> I'm thinking its probably a bad idea long term, just not sure what it
>> is short-term.
>>
>> I was thinking of just making scrnIndex for GPU screens have a
>> higher-bit set in them, and the logging would understand that and
>> strip it out, or maybe creating a new logging index that gets passed
>> to all the logging functions.
>>
>> However I was playing with using a combo or scrnIndex and isGPU to
>> denote a GPU screen, and it seems cleaner, but the big problem is
>> changing the logging function signatures is a major amount of work,
>> the API changes I've been making are moderate in comparison, but
>> everyone calls the logging functions from some really wierd places so
>> there would be a lot of audit.
>>
>> So I think thinking short-term, I just do the high-bit or start gpu
>> screens after MAXSCREENS, and make sure to never expose that
>> implementation detail to drivers, then once we are past the worst of
>> this we can contemplate some way to fix the logging interfaces.
>
> How about adding an interface that converts a screen pointer into a
> string of some sorts and use that in the logging functions?

Well we currently have one that turns the scrnIndex into a string
inside the logging functions
so just changing scrnIndex to ScrnInfoPtr would be fine with NULL
doing no string, but the sheer
number of callsites is overwhelming and keeping drivers building
against old/new servers would
also hurt, which are the main reasons I'd be punting on this for now.

Dave.