[PATCH glint v2] Make pm2's xv driver collect options like all other drivers.
Matt Turner
mattst88 at gmail.com
Sat Aug 13 14:00:19 PDT 2011
On Mon, Feb 28, 2011 at 7:08 PM, Matt Turner <mattst88 at gmail.com> wrote:
> On Sat, Dec 11, 2010 at 4:38 AM, Jesse Adkins <jesserayadkins at gmail.com> wrote:
>> The current method of argument collection is to collect options from different
>> ports of a VideoAdaptor record. Specifically, the ports had to be named
>> 'Input' for input options, and 'Output' for output options.
>>
>> This resulted in three groups of options, requiring people to know what
>> VideoAdaptor does, both of which were not documented in the man page. If you
>> forgot to create a VideoAdaptor port, then the xv driver would just not work.
>>
>> This patch makes the xv driver collect options from the screen, like every
>> single other driver. Input and Output prefixes are used for options where the
>> input and output ports have the same args (FramesPerSec, for example).
>> Documentation added for the change.
>>
>> This is a step toward getting rid of VideoAdaptor, since only glint uses it
>> (and is probably the only one to have used it).
>>
>> v2: Complain about Xv driver failing to load only if the user wanted Xv.
>> Don't use pGlint->Options, since glint is still initializing.
>>
>> Signed-off-by: Jesse Adkins <jesserayadkins at gmail.com>
>> ---
>> man/glint.man | 24 ++++++++++
>> src/pm2_video.c | 133 ++++++++++++++++++++++++-------------------------------
>> 2 files changed, 82 insertions(+), 75 deletions(-)
>>
>> diff --git a/man/glint.man b/man/glint.man
>> index 3bafcf4..3505e94 100644
>> --- a/man/glint.man
>> +++ b/man/glint.man
>> @@ -102,6 +102,30 @@ acceleration in general, but disables it for some special cases. Default: off.
>> .BI "Option \*qFireGL3000\*q \*q" boolean \*q
>> If you have a card of the same name, turn this on. Default: off.
>> .TP
>> +The Permedia 2 xv driver supports some additional options:
>> +.TP
>> +.BI "Option \*qDevice\*q \*q" string \*q
>> +A path to the Permedia 2 kernel driver. This is required for Xv support.
>
> Let's figure out what this actually means before we commit it. Was
> this referring to the now-defunct glint DRM? If so, do any of these
> options actually do anything?
>
>> +.TP
>> +.BI "Option \*qInputBuffers\*q \*q" integer \*q
>> +Sets the number of buffers for incoming data. Minimum of 1, max of 2.
>> +.TP
>> +.BI "Option \*qInputFramesPerSec\*q \*q" integer \*q
>> +Expected frames per second for incoming data.
>> +.TP
>> +.BI "Option \*qInputEncoding\*q \*q" string \*q
>> +The encoding that input data will have.
>> +.TP
>> +.BI "Option \*qOutputBuffers\*q \*q" integer \*q
>> +This should probably set the number of buffers for outgoing data. It actually
>> +does nothing.
>> +.TP
>> +.BI "Option \*qOutputFramesPerSec\*q \*q" integer \*q
>> +Expected frames per second for outgoing data.
>> +.TP
>> +.BI "Option \*qOutputEncoding\*q \*q" string \*q
>> +The encoding to put output data in.
>> +.TP
>> .SH "SEE ALSO"
>> __xservername__(__appmansuffix__), __xconfigfile__(__filemansuffix__), Xserver(__appmansuffix__), X(__miscmansuffix__)
>> .SH AUTHORS
>> diff --git a/src/pm2_video.c b/src/pm2_video.c
>> index 0c47d16..5fc2d23 100644
>> --- a/src/pm2_video.c
>> +++ b/src/pm2_video.c
>> @@ -199,28 +199,22 @@ static const Bool ColorBars = FALSE;
>>
>> typedef enum {
>> OPTION_DEVICE,
>> - OPTION_FPS,
>> - OPTION_BUFFERS,
>> - OPTION_ENCODING,
>> - OPTION_EXPOSE /* obsolete, ignored */
>> + OPTION_IN_FPS,
>> + OPTION_IN_BUFFERS,
>> + OPTION_IN_ENCODING,
>> + OPTION_OUT_FPS,
>> + OPTION_OUT_BUFFERS,
>> + OPTION_OUT_ENCODING,
>> } OptToken;
>>
>> -/* XXX These should be made const, and per-screen/adaptor copies processed. */
>> -static OptionInfoRec AdaptorOptions[] = {
>> +static const OptionInfoRec pm2Options[] = {
>> { OPTION_DEVICE, "Device", OPTV_STRING, {0}, FALSE },
>> - { -1, NULL, OPTV_NONE, {0}, FALSE }
>> -};
>> -static OptionInfoRec InputOptions[] = {
>> - { OPTION_BUFFERS, "Buffers", OPTV_INTEGER, {0}, FALSE },
>> - { OPTION_FPS, "FramesPerSec", OPTV_INTEGER, {0}, FALSE },
>> - { OPTION_ENCODING, "Encoding", OPTV_STRING, {0}, FALSE },
>> - { -1, NULL, OPTV_NONE, {0}, FALSE }
>> -};
>> -static OptionInfoRec OutputOptions[] = {
>> - { OPTION_BUFFERS, "Buffers", OPTV_INTEGER, {0}, FALSE },
>> - { OPTION_EXPOSE, "Expose", OPTV_BOOLEAN, {0}, FALSE },
>> - { OPTION_FPS, "FramesPerSec", OPTV_INTEGER, {0}, FALSE },
>> - { OPTION_ENCODING, "Encoding", OPTV_STRING, {0}, FALSE },
>> + { OPTION_IN_BUFFERS, "InputBuffers", OPTV_INTEGER, {0}, FALSE },
>> + { OPTION_IN_FPS, "InputFramesPerSec", OPTV_INTEGER, {0}, FALSE },
>> + { OPTION_IN_ENCODING, "InputEncoding", OPTV_STRING, {0}, FALSE },
>> + { OPTION_OUT_BUFFERS, "OutputBuffers", OPTV_INTEGER, {0}, FALSE },
>> + { OPTION_OUT_FPS, "OutputFramesPerSec", OPTV_INTEGER, {0}, FALSE },
>> + { OPTION_OUT_ENCODING, "OutputEncoding", OPTV_STRING, {0}, FALSE },
>> { -1, NULL, OPTV_NONE, {0}, FALSE }
>> };
>>
>> @@ -2964,12 +2958,13 @@ Permedia2VideoInit(ScreenPtr pScreen)
>> ScrnInfoPtr pScrn = xf86Screens[pScreen->myNum];
>> GLINTPtr pGlint = GLINTPTR(pScrn);
>> AdaptorPrivPtr pAPriv;
>> - pointer options[3];
>> DevUnion Private[PORTS];
>> XF86VideoAdaptorRec VAR[ADAPTORS];
>> XF86VideoAdaptorPtr VARPtrs[ADAPTORS];
>> Bool VideoIO = TRUE;
>> int i;
>> + /* Glint is still intializing, so pGlint->Options is off-limits. */
>> + OptionInfoPtr VidOpts;
>>
>> switch (pGlint->Chipset) {
>> case PCI_VENDOR_TI_CHIP_PERMEDIA2:
>> @@ -2981,64 +2976,50 @@ Permedia2VideoInit(ScreenPtr pScreen)
>> return;
>> }
>>
>> - options[0] = NULL; /* VideoAdaptor "input" subsection options */
>> - options[1] = NULL; /* VideoAdaptor "output" subsection options */
>> - options[2] = NULL; /* VideoAdaptor options */
>> -
>> - for (i = 0;; i++) {
>> - char *adaptor = NULL; /* receives VideoAdaptor section identifier */
>> -
>> - if (!options[0])
>> - options[0] = xf86FindXvOptions(pScreen->myNum, i, "input", &adaptor, options[2] ? NULL : &options[2]);
>> -
>> - if (!options[1])
>> - options[1] = xf86FindXvOptions(pScreen->myNum, i, "output", &adaptor, options[2] ? NULL : &options[2]);
>> -
>> - if (!adaptor) {
>> - if (!i) /* VideoAdaptor reference enables Xv vio driver */
>> - VideoIO = FALSE;
>> - break;
>> - } else if (options[0] && options[1])
>> - break;
>> - }
>> -
>> - if (VideoIO) {
>> - unsigned int temp;
>> - PCI_READ_LONG(pGlint->PciInfo, &temp, PCI_SUBSYSTEM_ID_REG);
>> - switch (temp) {
>> - case PCI_SUBSYSTEM_ID_WINNER_2000_P2A:
>> - case PCI_SUBSYSTEM_ID_WINNER_2000_P2C:
>> - case PCI_SUBSYSTEM_ID_GLORIA_SYNERGY_P2A:
>> - case PCI_SUBSYSTEM_ID_GLORIA_SYNERGY_P2C:
>> - break;
>> + xf86CollectOptions(pScrn, NULL);
>> + /* Process the options */
>> + if (!(VidOpts = malloc(sizeof(pm2Options))))
>> + return;
>>
>> - default:
>> - xf86DrvMsgVerb(pScrn->scrnIndex, X_PROBED, 1, "No Xv vio support for this board\n");
>> - VideoIO = FALSE;
>> + memcpy(VidOpts, pm2Options, sizeof(pm2Options));
>> +
>> + xf86ProcessOptions(pScrn->scrnIndex, pScrn->options, VidOpts);
>> + xf86ShowUnusedOptions(pScrn->scrnIndex, VidOpts);
>> +
>> + /* Don't complain about no Xv support unless they asked for Xv support.
>> + Assume they want Xv if OPTION_DEVICE is set, since that's required. */
>> + if (xf86IsOptionSet(VidOpts, OPTION_DEVICE)) {
>> + unsigned int temp;
>> + PCI_READ_LONG(pGlint->PciInfo, &temp, PCI_SUBSYSTEM_ID_REG);
>> + switch (temp) {
>> + case PCI_SUBSYSTEM_ID_WINNER_2000_P2A:
>> + case PCI_SUBSYSTEM_ID_WINNER_2000_P2C:
>> + case PCI_SUBSYSTEM_ID_GLORIA_SYNERGY_P2A:
>> + case PCI_SUBSYSTEM_ID_GLORIA_SYNERGY_P2C:
>> + break;
>> + default:
>> + VideoIO = FALSE;
>> + xf86DrvMsgVerb(pScrn->scrnIndex, X_PROBED, 1, "No Xv vio support for this board\n");
>> + }
>> }
>> + else
>> + /* Assume they don't, even if other options are set. */
>> + VideoIO = FALSE;
>> +
>> + if (pGlint->NoAccel && !VideoIO) {
>> + free(VidOpts);
>> + return;
>> }
>> - if (pGlint->NoAccel && !VideoIO)
>> - return;
>>
>> xf86DrvMsgVerb(pScrn->scrnIndex, X_INFO, 1, "Initializing Xv driver rev. 4\n");
>>
>> - if (VideoIO) {
>> - for (i = 0; i <= 2; i++) {
>> - xf86ProcessOptions(pScrn->scrnIndex, options[i],
>> - (i == 0) ? InputOptions :
>> - (i == 1) ? OutputOptions :
>> - AdaptorOptions);
>> -
>> - xf86ShowUnusedOptions(pScrn->scrnIndex, options[i]);
>> - }
>> -
>> - if (xf86IsOptionSet(AdaptorOptions, OPTION_DEVICE)) {
>> - if (!xvipcOpen(xf86GetOptValString(AdaptorOptions, OPTION_DEVICE), pScrn))
>> + if (VideoIO) {
>> + if (!xvipcOpen(xf86GetOptValString(VidOpts, OPTION_DEVICE), pScrn))
>> VideoIO = FALSE;
>> - }
>> }
>>
>> if (!(pAPriv = NewAdaptorPriv(pScrn, VideoIO))) {
>> + free(VidOpts);
>> xf86DrvMsg(pScrn->scrnIndex, X_ERROR, "Xv driver initialization failed\n");
>> return;
>> }
>> @@ -3046,14 +3027,14 @@ Permedia2VideoInit(ScreenPtr pScreen)
>> if (VideoIO) {
>> int n;
>>
>> - if (xf86GetOptValInteger(InputOptions, OPTION_BUFFERS, &n))
>> + if (xf86GetOptValInteger(VidOpts, OPTION_IN_BUFFERS, &n))
>> pAPriv->Port[0].BuffersRequested = CLAMP(n, 1, 2);
>> - if (xf86GetOptValInteger(InputOptions, OPTION_FPS, &n))
>> + if (xf86GetOptValInteger(VidOpts, OPTION_IN_FPS, &n))
>> pAPriv->Port[0].FramesPerSec = CLAMP(n, 1, 30);
>>
>> - if (xf86GetOptValInteger(OutputOptions, OPTION_BUFFERS, &n))
>> + if (xf86GetOptValInteger(VidOpts, OPTION_OUT_BUFFERS, &n))
>> pAPriv->Port[1].BuffersRequested = 1;
>> - if (xf86GetOptValInteger(OutputOptions, OPTION_FPS, &n))
>> + if (xf86GetOptValInteger(VidOpts, OPTION_OUT_FPS, &n))
>> pAPriv->Port[1].FramesPerSec = CLAMP(n, 1, 30);
>> }
>>
>> @@ -3073,7 +3054,7 @@ Permedia2VideoInit(ScreenPtr pScreen)
>> xf86InitFBManager(pScreen, &AvailFBArea);
>> }
>>
>> -#if 0
>> +#if defined(XFree86LOADER) && 0
>> if (xf86LoaderCheckSymbol("xf86InitLinearFBManagerRegion")) {
>> int last = pGlint->FbMapSize / (pScrn->bitsPerPixel / 8) - 1;
>> BoxRec AvailFBArea;
>> @@ -3178,14 +3159,14 @@ Permedia2VideoInit(ScreenPtr pScreen)
>> xf86DrvMsg(pScrn->scrnIndex, X_INFO, "Xv frontend scaler enabled\n");
>>
>> if (VideoIO) {
>> - if ((s = xf86GetOptValString(InputOptions, OPTION_ENCODING)))
>> + if ((s = xf86GetOptValString(VidOpts, OPTION_IN_ENCODING)))
>> for (i = 0; i < ENTRIES(InputVideoEncodings); i++)
>> if (!strncmp(s, InputVideoEncodings[i].name, strlen(s))) {
>> Permedia2SetPortAttribute(pScrn, xvEncoding, i, (pointer) &pAPriv->Port[0]);
>> break;
>> }
>>
>> - if ((s = xf86GetOptValString(OutputOptions, OPTION_ENCODING)))
>> + if ((s = xf86GetOptValString(VidOpts, OPTION_OUT_ENCODING)))
>> for (i = 0; i < ENTRIES(OutputVideoEncodings); i++)
>> if (!strncmp(s, OutputVideoEncodings[i].name, strlen(s))) {
>> Permedia2SetPortAttribute(pScrn, xvEncoding, i, (pointer) &pAPriv->Port[1]);
>> @@ -3199,4 +3180,6 @@ Permedia2VideoInit(ScreenPtr pScreen)
>> xf86DrvMsg(pScrn->scrnIndex, X_ERROR, "Xv initialization failed\n");
>> DeleteAdaptorPriv(pAPriv);
>> }
>> +
>> + free(VidOpts);
>> }
>> --
>> 1.7.1
>
> Any issues with this, Mark?
Jesse seems to have dropped off the xorg-devel mailing list, and I
don't think this patch will negatively affect anything, so I pushed
it.
Matt
More information about the xorg-devel
mailing list