diff mbox series

[V2,4/4] drm/crc: add pre_crc_read function

Message ID 20180821083858.26275-5-mahesh1.kumar@intel.com (mailing list archive)
State New, archived
Headers show
Series Improve crc-core driver interface | expand

Commit Message

Kumar, Mahesh Aug. 21, 2018, 8:38 a.m. UTC
This patch implements a callback function which will be called before
crc read. In this function driver can implement any preparation work
required for successfully reading CRC data.

Changes Since V1:
 - rebase

Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
---
 drivers/gpu/drm/drm_debugfs_crc.c |  8 ++++++++
 include/drm/drm_crtc.h            | 14 ++++++++++++++
 2 files changed, 22 insertions(+)

Comments

Rodrigo Vivi Aug. 21, 2018, 3:56 p.m. UTC | #1
On Tue, Aug 21, 2018 at 02:08:58PM +0530, Mahesh Kumar wrote:
> This patch implements a callback function which will be called before
> crc read. In this function driver can implement any preparation work
> required for successfully reading CRC data.
> 
> Changes Since V1:
>  - rebase
> 
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/drm_debugfs_crc.c |  8 ++++++++
>  include/drm/drm_crtc.h            | 14 ++++++++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c
> index 00e743153e94..e303c194c080 100644
> --- a/drivers/gpu/drm/drm_debugfs_crc.c
> +++ b/drivers/gpu/drm/drm_debugfs_crc.c
> @@ -274,6 +274,14 @@ static ssize_t crtc_crc_read(struct file *filep, char __user *user_buf,
>  		return 0;
>  	}
>  
> +	if (crtc->funcs->pre_crc_read) {
> +		ret = crtc->funcs->pre_crc_read(crtc);
> +		if (ret) {
> +			spin_unlock_irq(&crc->lock);
> +			return ret;
> +		}
> +	}
> +
>  	/* Nothing to read? */
>  	while (crtc_crc_data_count(crc) == 0) {
>  		if (filep->f_flags & O_NONBLOCK) {
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index b21437bc95bf..19c2bf63935e 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -761,6 +761,20 @@ struct drm_crtc_funcs {
>  	 */
>  	int (*verify_crc_source)(struct drm_crtc *crtc, const char *source,
>  				 size_t *values_cnt);
> +	/**
> +	 * @pre_crc_read:
> +	 *
> +	 * Driver callback for performing any preparation work required by
> +	 * driver before reading CRC
> +	 *
> +	 * This callback is optional if the driver does not support CRC
> +	 * generation or no prework is required before reading the crc
> +	 *
> +	 * RETURNS:
> +	 *
> +	 * 0 on success or a negative error code on failure.
> +	 */
> +	int (*pre_crc_read)(struct drm_crtc *crtc);
>  	/**
>  	 * @get_crc_sources:
>  	 *
> -- 
> 2.16.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Maarten Lankhorst Aug. 22, 2018, 10:37 a.m. UTC | #2
Op 21-08-18 om 10:38 schreef Mahesh Kumar:
> This patch implements a callback function which will be called before
> crc read. In this function driver can implement any preparation work
> required for successfully reading CRC data.
>
> Changes Since V1:
>  - rebase
>
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
After some discussion with David on whether this can be upstreamed, he NACK'd it.
Because there are no upstream users for this callback, it cannot be included upstream.
It would be removed by the first person to notice there are no in-kernel users using it.

For the first 3 patches:
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>  drivers/gpu/drm/drm_debugfs_crc.c |  8 ++++++++
>  include/drm/drm_crtc.h            | 14 ++++++++++++++
>  2 files changed, 22 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c
> index 00e743153e94..e303c194c080 100644
> --- a/drivers/gpu/drm/drm_debugfs_crc.c
> +++ b/drivers/gpu/drm/drm_debugfs_crc.c
> @@ -274,6 +274,14 @@ static ssize_t crtc_crc_read(struct file *filep, char __user *user_buf,
>  		return 0;
>  	}
>  
> +	if (crtc->funcs->pre_crc_read) {
> +		ret = crtc->funcs->pre_crc_read(crtc);
> +		if (ret) {
> +			spin_unlock_irq(&crc->lock);
> +			return ret;
> +		}
> +	}
> +
>  	/* Nothing to read? */
>  	while (crtc_crc_data_count(crc) == 0) {
>  		if (filep->f_flags & O_NONBLOCK) {
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index b21437bc95bf..19c2bf63935e 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -761,6 +761,20 @@ struct drm_crtc_funcs {
>  	 */
>  	int (*verify_crc_source)(struct drm_crtc *crtc, const char *source,
>  				 size_t *values_cnt);
> +	/**
> +	 * @pre_crc_read:
> +	 *
> +	 * Driver callback for performing any preparation work required by
> +	 * driver before reading CRC
> +	 *
> +	 * This callback is optional if the driver does not support CRC
> +	 * generation or no prework is required before reading the crc
> +	 *
> +	 * RETURNS:
> +	 *
> +	 * 0 on success or a negative error code on failure.
> +	 */
> +	int (*pre_crc_read)(struct drm_crtc *crtc);
>  	/**
>  	 * @get_crc_sources:
>  	 *
Rodrigo Vivi Aug. 22, 2018, 4:54 p.m. UTC | #3
On Wed, Aug 22, 2018 at 12:37:05PM +0200, Maarten Lankhorst wrote:
> Op 21-08-18 om 10:38 schreef Mahesh Kumar:
> > This patch implements a callback function which will be called before
> > crc read. In this function driver can implement any preparation work
> > required for successfully reading CRC data.
> >
> > Changes Since V1:
> >  - rebase
> >
> > Cc: dri-devel@lists.freedesktop.org
> > Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> After some discussion with David on whether this can be upstreamed, he NACK'd it.
> Because there are no upstream users for this callback, it cannot be included upstream.
> It would be removed by the first person to notice there are no in-kernel users using it.

It makes sense.... After I posted the review yesterday I started thinking exactly
about this case... if would get easily removed in a clean-up for obvious reasons...

> For the first 3 patches:
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

thanks. pushed all 3 to drm-misc-next.

> >  drivers/gpu/drm/drm_debugfs_crc.c |  8 ++++++++
> >  include/drm/drm_crtc.h            | 14 ++++++++++++++
> >  2 files changed, 22 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c
> > index 00e743153e94..e303c194c080 100644
> > --- a/drivers/gpu/drm/drm_debugfs_crc.c
> > +++ b/drivers/gpu/drm/drm_debugfs_crc.c
> > @@ -274,6 +274,14 @@ static ssize_t crtc_crc_read(struct file *filep, char __user *user_buf,
> >  		return 0;
> >  	}
> >  
> > +	if (crtc->funcs->pre_crc_read) {
> > +		ret = crtc->funcs->pre_crc_read(crtc);
> > +		if (ret) {
> > +			spin_unlock_irq(&crc->lock);
> > +			return ret;
> > +		}
> > +	}
> > +
> >  	/* Nothing to read? */
> >  	while (crtc_crc_data_count(crc) == 0) {
> >  		if (filep->f_flags & O_NONBLOCK) {
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > index b21437bc95bf..19c2bf63935e 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -761,6 +761,20 @@ struct drm_crtc_funcs {
> >  	 */
> >  	int (*verify_crc_source)(struct drm_crtc *crtc, const char *source,
> >  				 size_t *values_cnt);
> > +	/**
> > +	 * @pre_crc_read:
> > +	 *
> > +	 * Driver callback for performing any preparation work required by
> > +	 * driver before reading CRC
> > +	 *
> > +	 * This callback is optional if the driver does not support CRC
> > +	 * generation or no prework is required before reading the crc
> > +	 *
> > +	 * RETURNS:
> > +	 *
> > +	 * 0 on success or a negative error code on failure.
> > +	 */
> > +	int (*pre_crc_read)(struct drm_crtc *crtc);
> >  	/**
> >  	 * @get_crc_sources:
> >  	 *
> 
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_debugfs_crc.c b/drivers/gpu/drm/drm_debugfs_crc.c
index 00e743153e94..e303c194c080 100644
--- a/drivers/gpu/drm/drm_debugfs_crc.c
+++ b/drivers/gpu/drm/drm_debugfs_crc.c
@@ -274,6 +274,14 @@  static ssize_t crtc_crc_read(struct file *filep, char __user *user_buf,
 		return 0;
 	}
 
+	if (crtc->funcs->pre_crc_read) {
+		ret = crtc->funcs->pre_crc_read(crtc);
+		if (ret) {
+			spin_unlock_irq(&crc->lock);
+			return ret;
+		}
+	}
+
 	/* Nothing to read? */
 	while (crtc_crc_data_count(crc) == 0) {
 		if (filep->f_flags & O_NONBLOCK) {
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index b21437bc95bf..19c2bf63935e 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -761,6 +761,20 @@  struct drm_crtc_funcs {
 	 */
 	int (*verify_crc_source)(struct drm_crtc *crtc, const char *source,
 				 size_t *values_cnt);
+	/**
+	 * @pre_crc_read:
+	 *
+	 * Driver callback for performing any preparation work required by
+	 * driver before reading CRC
+	 *
+	 * This callback is optional if the driver does not support CRC
+	 * generation or no prework is required before reading the crc
+	 *
+	 * RETURNS:
+	 *
+	 * 0 on success or a negative error code on failure.
+	 */
+	int (*pre_crc_read)(struct drm_crtc *crtc);
 	/**
 	 * @get_crc_sources:
 	 *