[PATCH] Make pm2's xv driver collect options like all other drivers.
Matt Turner
mattst88 at gmail.com
Sat Dec 4 08:33:49 PST 2010
On Fri, Nov 12, 2010 at 3:48 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).
>
> Signed-off-by: Jesse Adkins <jesserayadkins at gmail.com>
> ---
> man/glint.man | 24 +++++++++++
> src/pm2_video.c | 115 ++++++++++++++++++++++---------------------------------
> 2 files changed, 70 insertions(+), 69 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.
This looks like it needs to be changed. I don't know if this is a
relic of the days when we had glint DRM or what.
> +.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 df10bea..82eab32 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,7 +2958,6 @@ 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];
> @@ -2981,64 +2974,48 @@ 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;
> -
> - default:
> - xf86DrvMsgVerb(pScrn->scrnIndex, X_PROBED, 1, "No Xv vio support for this board\n");
> - VideoIO = FALSE;
> + {
> + /* Check for vio support. Input+Output options will be ignored when
> + there's no vio support. */
> + 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");
> }
> - }
> +
> 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);
> + xf86CollectOptions(pScrn, NULL);
> + /* Process the options */
> + if (!(pGlint->Options = malloc(sizeof(pm2Options))))
> + return;
We have a malloc here.
> +
> + memcpy(pGlint->Options, pm2Options, sizeof(pm2Options));
>
> - xf86ShowUnusedOptions(pScrn->scrnIndex, options[i]);
> + xf86ProcessOptions(pScrn->scrnIndex, pScrn->options, pGlint->Options);
> + xf86ShowUnusedOptions(pScrn->scrnIndex, pGlint->Options);
> }
>
> - if (xf86IsOptionSet(AdaptorOptions, OPTION_DEVICE)) {
> - if (!xvipcOpen(xf86GetOptValString(AdaptorOptions, OPTION_DEVICE), pScrn))
> + if (xf86IsOptionSet(pm2Options, OPTION_DEVICE)) {
> + if (!xvipcOpen(xf86GetOptValString(pm2Options, OPTION_DEVICE), pScrn))
> VideoIO = FALSE;
> }
> }
>
> if (!(pAPriv = NewAdaptorPriv(pScrn, VideoIO))) {
> + if (VideoIO)
> + free(pGlint->Options);
And a conditional free. Are we missing another free somewhere else?
> xf86DrvMsg(pScrn->scrnIndex, X_ERROR, "Xv driver initialization failed\n");
> return;
> }
> @@ -3046,14 +3023,14 @@ Permedia2VideoInit(ScreenPtr pScreen)
> if (VideoIO) {
> int n;
>
> - if (xf86GetOptValInteger(InputOptions, OPTION_BUFFERS, &n))
> + if (xf86GetOptValInteger(pm2Options, OPTION_IN_BUFFERS, &n))
> pAPriv->Port[0].BuffersRequested = CLAMP(n, 1, 2);
> - if (xf86GetOptValInteger(InputOptions, OPTION_FPS, &n))
> + if (xf86GetOptValInteger(pm2Options, OPTION_IN_FPS, &n))
> pAPriv->Port[0].FramesPerSec = CLAMP(n, 1, 30);
>
> - if (xf86GetOptValInteger(OutputOptions, OPTION_BUFFERS, &n))
> + if (xf86GetOptValInteger(pm2Options, OPTION_OUT_BUFFERS, &n))
> pAPriv->Port[1].BuffersRequested = 1;
> - if (xf86GetOptValInteger(OutputOptions, OPTION_FPS, &n))
> + if (xf86GetOptValInteger(pm2Options, OPTION_OUT_FPS, &n))
> pAPriv->Port[1].FramesPerSec = CLAMP(n, 1, 30);
> }
>
> @@ -3178,14 +3155,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(pm2Options, 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(pm2Options, 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]);
> --
More information about the xorg-devel
mailing list