[PATCH replace] DDX/fbdevhw: Disable FBIOBLANK ioctl if not supported
Egbert Eich
e4t at freenet.de
Sat Oct 5 04:01:09 PDT 2013
On Fri, Oct 04, 2013 at 02:32:30PM -0700, Keith Packard wrote:
> Egbert Eich <eich at freedesktop.org> writes:
>
> > Some ioctls may not be supported by the kernel however their failure
> > is non-fatal to the driver. Unfortunately we only know once we try
> > to execute the ioctl however the sematics of the fbdev driver API
> > doesn't allow upper layers to disable the call.
> > Instead of changing the fbdevHW driver API just disable the call to
> > this ioctl on the module level when detecting such a case.
>
> I'd not bother with remembering whether the ioctls were supported and
> just call every time, ignoring any errors and not printing
> anything ever.
Originally this patch was ment to prevent a message from being printed
every time the server tried to blank the screen.
Too many users were posting useless log files thinking this (harmless)
message had something to do with the issue they were seeing.
This then outgrew the original purpose - now since this code gets touched
anyway we could just as well fix the error handling.
>
> If you want an error message on the first call, I think just having a
> static boolean within the lexical scope of the function would be simpler
> than adding the per-device bitmask, enum of unsupported ioctl names etc.
>
> Of course, I don't care *that* deeply either, so if this is really what
> you want...
>
> > + if (-1 == ioctl(fPtr->fd, FBIOBLANK, (void *) fbmode)) {
> > + switch (errno) {
> > + case EAGAIN:
> > + case EINTR:
> > + case ERESTART:
> > + xf86DrvMsg(pScrn->scrnIndex, X_INFO,
> > + "FBIOBLANK: %s\n", strerror(errno));
> > + break;
>
> Presumably some of these values should cause the driver to retry the
> operation?
>
Yeah, we should probably loop and retry on EINTR and ERESTART.
Will change this and resend.
Cheers,
Egbert.
More information about the xorg-devel
mailing list