| Message ID | 20190703150330.21992-1-Liviu.Dudau@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | drm/drm_debugfs_crc.c: Document that .verify_crc_source vfunc is required for enabling CRC support. | expand |
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@ffwll.ch> > Signed-off-by: Liviu Dudau <liviu.dudau@arm.com> Reviewed-by: Daniel Vetter <daniel.vetter@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. -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 >
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@ffwll.ch> > > Signed-off-by: Liviu Dudau <liviu.dudau@arm.com> > > Reviewed-by: Daniel Vetter <daniel.vetter@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? 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
On Wed, Jul 3, 2019 at 5:59 PM Liviu Dudau <Liviu.Dudau@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@ffwll.ch> > > > Signed-off-by: Liviu Dudau <liviu.dudau@arm.com> > > > > Reviewed-by: Daniel Vetter <daniel.vetter@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! / > --------------- > ¯\_(ツ)_/¯
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)
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@ffwll.ch> Signed-off-by: Liviu Dudau <liviu.dudau@arm.com> --- drivers/gpu/drm/drm_debugfs_crc.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)