[PATCH] drm/drm_debugfs_crc.c: Document that .verify_crc_source vfunc is required for enabling CRC support.

[Daniel Vetter]

On Wed, Jul 3, 2019 at 5:59 PM Liviu Dudau <Liviu.Dudau at arm.com> wrote:
>
> On Wed, Jul 03, 2019 at 05:21:20PM +0200, Daniel Vetter wrote:
> > On Wed, Jul 03, 2019 at 04:03:30PM +0100, Liviu Dudau wrote:
> > > drm_debugfs_crtc_crc_add() function checks that both .set_crc_source and
> > > .verify_crc_source hooks are provided before enabling debugfs support for
> > > reading per-frame CRC data. Make that explicit in the documentation.
> > >
> > > Cc: Daniel Vetter <daniel at ffwll.ch>
> > > Signed-off-by: Liviu Dudau <liviu.dudau at arm.com>
> >
> > Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> >
> > I think we have some refactoring room here if we make verify_crc_source
> > optional (and only allow "auto" for that case). But would only remove like
> > 3-4 implementations, so I guess that's for when the next trivial one shows
> > up.
>
> I'm preparing a patch to add CRC support for Komeda, does this means I need
> to look at that refactoring?

If all you do is add support for the "auto" source, then I think that
would be nice. Shouldn't be really more work than another copypasta
version, and then if you do the 3 or so patches to remove the copies
from drivers you also have people you can volunteer to review the
entire thing :-)
-Daniel

>
> Best regards,
> Liviu
>
>
> > -Daniel
> >
> > > ---
> > >  drivers/gpu/drm/drm_debugfs_crc.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c
> > > index 7ca486d750e9..6604ed223160 100644
> > > --- a/drivers/gpu/drm/drm_debugfs_crc.c
> > > +++ b/drivers/gpu/drm/drm_debugfs_crc.c
> > > @@ -66,9 +66,9 @@
> > >   * the reported CRCs of frames that should have the same contents.
> > >   *
> > >   * On the driver side the implementation effort is minimal, drivers only need to
> > > - * implement &drm_crtc_funcs.set_crc_source. The debugfs files are automatically
> > > - * set up if that vfunc is set. CRC samples need to be captured in the driver by
> > > - * calling drm_crtc_add_crc_entry().
> > > + * implement &drm_crtc_funcs.set_crc_source and &drm_crtc_funcs.verify_crc_source.
> > > + * The debugfs files are automatically set up if those vfuncs are set. CRC samples
> > > + * need to be captured in the driver by calling drm_crtc_add_crc_entry().
> > >   */
> > >
> > >  static int crc_control_show(struct seq_file *m, void *data)
> > > --
> > > 2.21.0
> > >
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
>
> --
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch