[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