[PATCH 00/10] drm/vkms: rework crc worker

[Daniel Vetter]

On Wed, Jun 19, 2019 at 12:25 AM Rodrigo Siqueira
<rodrigosiqueiramelo at gmail.com> wrote:
>
> On Tue, Jun 18, 2019 at 7:08 PM Daniel Vetter <daniel at ffwll.ch> wrote:
> >
> > On Wed, Jun 19, 2019 at 12:06 AM Daniel Vetter <daniel at ffwll.ch> wrote:
> > >
> > > On Tue, Jun 18, 2019 at 11:54 PM Rodrigo Siqueira
> > > <rodrigosiqueiramelo at gmail.com> wrote:
> > > > Finally, not related with this patchset, can I apply the patch
> > > > “drm/drm_vblank: Change EINVAL by the correct errno” [1] or do I need
> > > > more SoB? I’ll also apply Oleg patch (drm/vkms: add crc sources list).
> > > >
> > > > 1. https://patchwork.freedesktop.org/patch/310006/?series=50697&rev=4
> > >
> > > If you want get some acks from igt maintainers (those patches landed
> > > now, right), but this is good enough.
> >
> > Oh wait correction: My review is conditional on you changing that one
> > thing. So needs another version. Since this is a functional change imo
> > too much to fix up while applying.
>
> In your comment you said:
>
>   >   if (vblwait->request.type & _DRM_VBLANK_SIGNAL)
>   > - return -EINVAL;
>   > + return -EOPNOTSUPP;
>
>   Not sure we want EINVAL here, that's kinda a "parameters are wrong"
>   version too. With that changed:
>
> I think I did not got your point here, sorry for that... so, do you
> want that I change EOPNOTSUPP by EINVAL in the above code?

Oops, that was wrong. I meant to say that I don't see why we should
use EOPNOTSUPP here, the EINVAL indicating a wrong argument seems more
fitting to me. It's been pretty much forever (if we every supported
this) that vblank signal worked on linux. Ok, did a quick check, it
died in 2009. That's before the kms stuff landed, there's definitely
no userspace around anymore that ever expected this to work :-) Hence
why I think EINVAL is more fitting ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch