On Tue, Aug 21, 2018 at 09:53:11AM +0530, Kumar, Mahesh wrote:
> Hi Haneen,
>
>
> On 8/21/2018 4:09 AM, Haneen Mohammed wrote:
> > On Tue, Aug 14, 2018 at 08:31:03AM +0530, Mahesh Kumar wrote:
> > > This patch implements "verify_crc_source" callback function for
> > > Virtual KMS drm driver.
> > >
> > > Changes Since V1:
> > > - update values_cnt in verify_crc_source
> > >
> > > Cc: Haneen Mohammed <hamohammed...@gmail.com>
> > > Signed-off-by: Mahesh Kumar <mahesh1.ku...@intel.com>
> > > ---
> > > drivers/gpu/drm/vkms/vkms_crc.c | 38
> > > ++++++++++++++++++++++++++++++++++----
> > > drivers/gpu/drm/vkms/vkms_crtc.c | 1 +
> > > drivers/gpu/drm/vkms/vkms_drv.h | 2 ++
> > > 3 files changed, 37 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/vkms/vkms_crc.c
> > > b/drivers/gpu/drm/vkms/vkms_crc.c
> > > index 37d717f38e3c..b2a484b4e2ad 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_crc.c
> > > +++ b/drivers/gpu/drm/vkms/vkms_crc.c
> > > @@ -70,6 +70,37 @@ void vkms_crc_work_handle(struct work_struct *work)
> > > drm_crtc_add_crc_entry(crtc, true, crtc_state->n_frame, &crc32);
> > > }
> > > +static int vkms_crc_parse_source(const char *src_name, bool *enabled)
> > > +{
> > > + int ret = 0;
> > > +
> > > + if (!src_name) {
> > > + *enabled = false;
> > > + } else if (strcmp(src_name, "auto") == 0) {
> > > + *enabled = true;
> > > + } else {
> > > + *enabled = false;
> > > + ret = -EINVAL;
> > > + }
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +int vkms_verify_crc_source(struct drm_crtc *crtc, const char *src_name,
> > > + size_t *values_cnt)
> > > +{
> > > + bool enabled;
> > > +
> > > + if (vkms_crc_parse_source(src_name, &enabled) < 0) {
> > > + DRM_DEBUG_DRIVER("unknown source %s\n", src_name);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + *values_cnt = 1;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
> > > size_t *values_cnt)
> > > {
> > > @@ -78,10 +109,9 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const
> > > char *src_name,
> > > unsigned long flags;
> > > int ret = 0;
> > > - if (src_name && strcmp(src_name, "auto") == 0)
> > > - enabled = true;
> > > - else if (src_name)
> > > - ret = -EINVAL;
> > > + ret = vkms_crc_parse_source(src_name, &enabled);
> > > + if (ret)
> > > + return ret;
> > I think the return value after vkms_crc_parse_source should be called
> > once instead at the end of vkms_set_crc_source otherwise crc_enabled
> > won't be updated?
> Here I'm assuming if src_name is wrong we don't need to proceed further, let
> the CRC generation to continue whatever it was doing (enabled or disabled).
> But anyway that is changing the original design, will remove above return to
> keep the behavior same.
>
> -Mahesh
hm I'm not really sure if my approach was the right way, but if it wasn't
then I think maybe this change would be better if it was either in a separate
patch or
here with more details
Thank you,
Haneen
> >
> > - Haneen
> >
> > > *values_cnt = 1;
> > > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c
> > > b/drivers/gpu/drm/vkms/vkms_crtc.c
> > > index bfe6e0312cc4..9d0b1a325a78 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> > > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> > > @@ -140,6 +140,7 @@ static const struct drm_crtc_funcs vkms_crtc_funcs = {
> > > .enable_vblank = vkms_enable_vblank,
> > > .disable_vblank = vkms_disable_vblank,
> > > .set_crc_source = vkms_set_crc_source,
> > > + .verify_crc_source = vkms_verify_crc_source,
> > > };
> > > static void vkms_crtc_atomic_enable(struct drm_crtc *crtc,
> > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h
> > > b/drivers/gpu/drm/vkms/vkms_drv.h
> > > index f156c930366a..090c5e4f5544 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_drv.h
> > > +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> > > @@ -125,6 +125,8 @@ void vkms_gem_vunmap(struct drm_gem_object *obj);
> > > /* CRC Support */
> > > int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name,
> > > size_t *values_cnt);
> > > +int vkms_verify_crc_source(struct drm_crtc *crtc, const char
> > > *source_name,
> > > + size_t *values_cnt);
> > > void vkms_crc_work_handle(struct work_struct *work);
> > > #endif /* _VKMS_DRV_H_ */
> > > --
> > > 2.16.2
> > >
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
