diff mbox series

[01/23] drm: Add get_scanout_position() to struct drm_crtc_helper_funcs

Message ID 20200110092127.27847-2-tzimmermann@suse.de (mailing list archive)
State Superseded
Headers show
Series drm: Clean up VBLANK callbacks in struct drm_driver | expand

Commit Message

Thomas Zimmermann Jan. 10, 2020, 9:21 a.m. UTC
The new callback get_scanout_position() reads the current location of
the scanout process. The operation is currentyl located in struct
drm_driver, but really belongs to the CRTC. Drivers will be converted
in separate patches.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_vblank.c             | 24 ++++++++----
 include/drm/drm_drv.h                    |  7 +---
 include/drm/drm_modeset_helper_vtables.h | 47 ++++++++++++++++++++++++
 3 files changed, 65 insertions(+), 13 deletions(-)

Comments

Jani Nikula Jan. 10, 2020, 10:24 a.m. UTC | #1
On Fri, 10 Jan 2020, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> The new callback get_scanout_position() reads the current location of
> the scanout process. The operation is currentyl located in struct
> drm_driver, but really belongs to the CRTC. Drivers will be converted
> in separate patches.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/drm_vblank.c             | 24 ++++++++----
>  include/drm/drm_drv.h                    |  7 +---
>  include/drm/drm_modeset_helper_vtables.h | 47 ++++++++++++++++++++++++
>  3 files changed, 65 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 1659b13b178c..c12f0b333e14 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -30,6 +30,7 @@
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_drv.h>
>  #include <drm/drm_framebuffer.h>
> +#include <drm/drm_modeset_helper_vtables.h>
>  #include <drm/drm_print.h>
>  #include <drm/drm_vblank.h>
>  
> @@ -590,7 +591,7 @@ EXPORT_SYMBOL(drm_calc_timestamping_constants);
>   * Implements calculation of exact vblank timestamps from given drm_display_mode
>   * timings and current video scanout position of a CRTC. This can be directly
>   * used as the &drm_driver.get_vblank_timestamp implementation of a kms driver
> - * if &drm_driver.get_scanout_position is implemented.
> + * if &drm_crtc_helper_funcs.get_scanout_position is implemented.
>   *
>   * The current implementation only handles standard video modes. For double scan
>   * and interlaced modes the driver is supposed to adjust the hardware mode
> @@ -632,8 +633,9 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
>  	}
>  
>  	/* Scanout position query not supported? Should not happen. */
> -	if (!dev->driver->get_scanout_position) {
> -		DRM_ERROR("Called from driver w/o get_scanout_position()!?\n");
> +	if (!dev->driver->get_scanout_position ||
> +	    !crtc->helper_private->get_scanout_position) {

ITYM s/||/&&/

BR,
Jani.


> +		DRM_ERROR("Called from CRTC w/o get_scanout_position()!?\n");
>  		return false;
>  	}
>  
> @@ -664,11 +666,17 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
>  		 * Get vertical and horizontal scanout position vpos, hpos,
>  		 * and bounding timestamps stime, etime, pre/post query.
>  		 */
> -		vbl_status = dev->driver->get_scanout_position(dev, pipe,
> -							       in_vblank_irq,
> -							       &vpos, &hpos,
> -							       &stime, &etime,
> -							       mode);
> +		if (crtc->helper_private->get_scanout_position) {
> +			vbl_status =
> +				crtc->helper_private->get_scanout_position(
> +					crtc, in_vblank_irq, &vpos, &hpos,
> +					&stime, &etime, mode);
> +		} else {
> +			vbl_status =
> +				dev->driver->get_scanout_position(
> +					dev, pipe, in_vblank_irq, &vpos,
> +					&hpos, &stime, &etime, mode);
> +		}
>  
>  		/* Return as no-op if scanout query unsupported or failed. */
>  		if (!vbl_status) {
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index cf13470810a5..d0049e5786fc 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -362,11 +362,8 @@ struct drm_driver {
>  	 * True on success, false if a reliable scanout position counter could
>  	 * not be read out.
>  	 *
> -	 * FIXME:
> -	 *
> -	 * Since this is a helper to implement @get_vblank_timestamp, we should
> -	 * move it to &struct drm_crtc_helper_funcs, like all the other
> -	 * helper-internal hooks.
> +	 * This is deprecated and should not be used by new drivers.
> +	 * Use &drm_crtc_helper_funcs.get_scanout_position instead.
>  	 */
>  	bool (*get_scanout_position) (struct drm_device *dev, unsigned int pipe,
>  				      bool in_vblank_irq, int *vpos, int *hpos,
> diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
> index 5a87f1bd7a3f..e398512bfd5f 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -450,6 +450,53 @@ struct drm_crtc_helper_funcs {
>  	 */
>  	void (*atomic_disable)(struct drm_crtc *crtc,
>  			       struct drm_crtc_state *old_crtc_state);
> +
> +	/**
> +	 * @get_scanout_position:
> +	 *
> +	 * Called by vblank timestamping code.
> +	 *
> +	 * Returns the current display scanout position from a CRTC and an
> +	 * optional accurate ktime_get() timestamp of when the position was
> +	 * measured. Note that this is a helper callback which is only used
> +	 * if a driver uses drm_calc_vbltimestamp_from_scanoutpos() for the
> +	 * @drm_driver.get_vblank_timestamp callback.
> +	 *
> +	 * Parameters:
> +	 *
> +	 * crtc:
> +	 *     The CRTC.
> +	 * in_vblank_irq:
> +	 *     True when called from drm_crtc_handle_vblank(). Some drivers
> +	 *     need to apply some workarounds for gpu-specific vblank irq
> +	 *     quirks if the flag is set.
> +	 * vpos:
> +	 *     Target location for current vertical scanout position.
> +	 * hpos:
> +	 *     Target location for current horizontal scanout position.
> +	 * stime:
> +	 *     Target location for timestamp taken immediately before
> +	 *     scanout position query. Can be NULL to skip timestamp.
> +	 * etime:
> +	 *     Target location for timestamp taken immediately after
> +	 *     scanout position query. Can be NULL to skip timestamp.
> +	 * mode:
> +	 *     Current display timings.
> +	 *
> +	 * Returns vpos as a positive number while in active scanout area.
> +	 * Returns vpos as a negative number inside vblank, counting the number
> +	 * of scanlines to go until end of vblank, e.g., -1 means "one scanline
> +	 * until start of active scanout / end of vblank."
> +	 *
> +	 * Returns:
> +	 *
> +	 * True on success, false if a reliable scanout position counter could
> +	 * not be read out.
> +	 */
> +	bool (*get_scanout_position)(struct drm_crtc *crtc,
> +				     bool in_vblank_irq, int *vpos, int *hpos,
> +				     ktime_t *stime, ktime_t *etime,
> +				     const struct drm_display_mode *mode);
>  };
>  
>  /**
Thomas Zimmermann Jan. 15, 2020, 7:31 a.m. UTC | #2
Hi

Am 14.01.20 um 16:31 schrieb Yannick FERTRE:
> Thanks for the patch.
> 
> Tested-by: Yannick Fertré <yannick.fertre@st.com> 

Thanks for testing all these patches.

Best regards
Thomas

> 
> BR
> Yannick Fertré
> 
> 
> On 1/10/20 10:21 AM, Thomas Zimmermann wrote:
>> The new callback get_scanout_position() reads the current location of
>> the scanout process. The operation is currentyl located in struct
>> drm_driver, but really belongs to the CRTC. Drivers will be converted
>> in separate patches.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>  drivers/gpu/drm/drm_vblank.c             | 24 ++++++++----
>>  include/drm/drm_drv.h                    |  7 +---
>>  include/drm/drm_modeset_helper_vtables.h | 47 ++++++++++++++++++++++++
>>  3 files changed, 65 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
>> index 1659b13b178c..c12f0b333e14 100644
>> --- a/drivers/gpu/drm/drm_vblank.c
>> +++ b/drivers/gpu/drm/drm_vblank.c
>> @@ -30,6 +30,7 @@
>>  #include <drm/drm_crtc.h>
>>  #include <drm/drm_drv.h>
>>  #include <drm/drm_framebuffer.h>
>> +#include <drm/drm_modeset_helper_vtables.h>
>>  #include <drm/drm_print.h>
>>  #include <drm/drm_vblank.h>
>>  
>> @@ -590,7 +591,7 @@ EXPORT_SYMBOL(drm_calc_timestamping_constants);
>>   * Implements calculation of exact vblank timestamps from given drm_display_mode
>>   * timings and current video scanout position of a CRTC. This can be directly
>>   * used as the &drm_driver.get_vblank_timestamp implementation of a kms driver
>> - * if &drm_driver.get_scanout_position is implemented.
>> + * if &drm_crtc_helper_funcs.get_scanout_position is implemented.
>>   *
>>   * The current implementation only handles standard video modes. For double scan
>>   * and interlaced modes the driver is supposed to adjust the hardware mode
>> @@ -632,8 +633,9 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
>>  	}
>>  
>>  	/* Scanout position query not supported? Should not happen. */
>> -	if (!dev->driver->get_scanout_position) {
>> -		DRM_ERROR("Called from driver w/o get_scanout_position()!?\n");
>> +	if (!dev->driver->get_scanout_position ||
>> +	    !crtc->helper_private->get_scanout_position) {
>> +		DRM_ERROR("Called from CRTC w/o get_scanout_position()!?\n");
>>  		return false;
>>  	}
>>  
>> @@ -664,11 +666,17 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
>>  		 * Get vertical and horizontal scanout position vpos, hpos,
>>  		 * and bounding timestamps stime, etime, pre/post query.
>>  		 */
>> -		vbl_status = dev->driver->get_scanout_position(dev, pipe,
>> -							       in_vblank_irq,
>> -							       &vpos, &hpos,
>> -							       &stime, &etime,
>> -							       mode);
>> +		if (crtc->helper_private->get_scanout_position) {
>> +			vbl_status =
>> +				crtc->helper_private->get_scanout_position(
>> +					crtc, in_vblank_irq, &vpos, &hpos,
>> +					&stime, &etime, mode);
>> +		} else {
>> +			vbl_status =
>> +				dev->driver->get_scanout_position(
>> +					dev, pipe, in_vblank_irq, &vpos,
>> +					&hpos, &stime, &etime, mode);
>> +		}
>>  
>>  		/* Return as no-op if scanout query unsupported or failed. */
>>  		if (!vbl_status) {
>> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
>> index cf13470810a5..d0049e5786fc 100644
>> --- a/include/drm/drm_drv.h
>> +++ b/include/drm/drm_drv.h
>> @@ -362,11 +362,8 @@ struct drm_driver {
>>  	 * True on success, false if a reliable scanout position counter could
>>  	 * not be read out.
>>  	 *
>> -	 * FIXME:
>> -	 *
>> -	 * Since this is a helper to implement @get_vblank_timestamp, we should
>> -	 * move it to &struct drm_crtc_helper_funcs, like all the other
>> -	 * helper-internal hooks.
>> +	 * This is deprecated and should not be used by new drivers.
>> +	 * Use &drm_crtc_helper_funcs.get_scanout_position instead.
>>  	 */
>>  	bool (*get_scanout_position) (struct drm_device *dev, unsigned int pipe,
>>  				      bool in_vblank_irq, int *vpos, int *hpos,
>> diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
>> index 5a87f1bd7a3f..e398512bfd5f 100644
>> --- a/include/drm/drm_modeset_helper_vtables.h
>> +++ b/include/drm/drm_modeset_helper_vtables.h
>> @@ -450,6 +450,53 @@ struct drm_crtc_helper_funcs {
>>  	 */
>>  	void (*atomic_disable)(struct drm_crtc *crtc,
>>  			       struct drm_crtc_state *old_crtc_state);
>> +
>> +	/**
>> +	 * @get_scanout_position:
>> +	 *
>> +	 * Called by vblank timestamping code.
>> +	 *
>> +	 * Returns the current display scanout position from a CRTC and an
>> +	 * optional accurate ktime_get() timestamp of when the position was
>> +	 * measured. Note that this is a helper callback which is only used
>> +	 * if a driver uses drm_calc_vbltimestamp_from_scanoutpos() for the
>> +	 * @drm_driver.get_vblank_timestamp callback.
>> +	 *
>> +	 * Parameters:
>> +	 *
>> +	 * crtc:
>> +	 *     The CRTC.
>> +	 * in_vblank_irq:
>> +	 *     True when called from drm_crtc_handle_vblank(). Some drivers
>> +	 *     need to apply some workarounds for gpu-specific vblank irq
>> +	 *     quirks if the flag is set.
>> +	 * vpos:
>> +	 *     Target location for current vertical scanout position.
>> +	 * hpos:
>> +	 *     Target location for current horizontal scanout position.
>> +	 * stime:
>> +	 *     Target location for timestamp taken immediately before
>> +	 *     scanout position query. Can be NULL to skip timestamp.
>> +	 * etime:
>> +	 *     Target location for timestamp taken immediately after
>> +	 *     scanout position query. Can be NULL to skip timestamp.
>> +	 * mode:
>> +	 *     Current display timings.
>> +	 *
>> +	 * Returns vpos as a positive number while in active scanout area.
>> +	 * Returns vpos as a negative number inside vblank, counting the number
>> +	 * of scanlines to go until end of vblank, e.g., -1 means "one scanline
>> +	 * until start of active scanout / end of vblank."
>> +	 *
>> +	 * Returns:
>> +	 *
>> +	 * True on success, false if a reliable scanout position counter could
>> +	 * not be read out.
>> +	 */
>> +	bool (*get_scanout_position)(struct drm_crtc *crtc,
>> +				     bool in_vblank_irq, int *vpos, int *hpos,
>> +				     ktime_t *stime, ktime_t *etime,
>> +				     const struct drm_display_mode *mode);
>>  };
>>  
>>  /**
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index 1659b13b178c..c12f0b333e14 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -30,6 +30,7 @@ 
 #include <drm/drm_crtc.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_framebuffer.h>
+#include <drm/drm_modeset_helper_vtables.h>
 #include <drm/drm_print.h>
 #include <drm/drm_vblank.h>
 
@@ -590,7 +591,7 @@  EXPORT_SYMBOL(drm_calc_timestamping_constants);
  * Implements calculation of exact vblank timestamps from given drm_display_mode
  * timings and current video scanout position of a CRTC. This can be directly
  * used as the &drm_driver.get_vblank_timestamp implementation of a kms driver
- * if &drm_driver.get_scanout_position is implemented.
+ * if &drm_crtc_helper_funcs.get_scanout_position is implemented.
  *
  * The current implementation only handles standard video modes. For double scan
  * and interlaced modes the driver is supposed to adjust the hardware mode
@@ -632,8 +633,9 @@  bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
 	}
 
 	/* Scanout position query not supported? Should not happen. */
-	if (!dev->driver->get_scanout_position) {
-		DRM_ERROR("Called from driver w/o get_scanout_position()!?\n");
+	if (!dev->driver->get_scanout_position ||
+	    !crtc->helper_private->get_scanout_position) {
+		DRM_ERROR("Called from CRTC w/o get_scanout_position()!?\n");
 		return false;
 	}
 
@@ -664,11 +666,17 @@  bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
 		 * Get vertical and horizontal scanout position vpos, hpos,
 		 * and bounding timestamps stime, etime, pre/post query.
 		 */
-		vbl_status = dev->driver->get_scanout_position(dev, pipe,
-							       in_vblank_irq,
-							       &vpos, &hpos,
-							       &stime, &etime,
-							       mode);
+		if (crtc->helper_private->get_scanout_position) {
+			vbl_status =
+				crtc->helper_private->get_scanout_position(
+					crtc, in_vblank_irq, &vpos, &hpos,
+					&stime, &etime, mode);
+		} else {
+			vbl_status =
+				dev->driver->get_scanout_position(
+					dev, pipe, in_vblank_irq, &vpos,
+					&hpos, &stime, &etime, mode);
+		}
 
 		/* Return as no-op if scanout query unsupported or failed. */
 		if (!vbl_status) {
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index cf13470810a5..d0049e5786fc 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -362,11 +362,8 @@  struct drm_driver {
 	 * True on success, false if a reliable scanout position counter could
 	 * not be read out.
 	 *
-	 * FIXME:
-	 *
-	 * Since this is a helper to implement @get_vblank_timestamp, we should
-	 * move it to &struct drm_crtc_helper_funcs, like all the other
-	 * helper-internal hooks.
+	 * This is deprecated and should not be used by new drivers.
+	 * Use &drm_crtc_helper_funcs.get_scanout_position instead.
 	 */
 	bool (*get_scanout_position) (struct drm_device *dev, unsigned int pipe,
 				      bool in_vblank_irq, int *vpos, int *hpos,
diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h
index 5a87f1bd7a3f..e398512bfd5f 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -450,6 +450,53 @@  struct drm_crtc_helper_funcs {
 	 */
 	void (*atomic_disable)(struct drm_crtc *crtc,
 			       struct drm_crtc_state *old_crtc_state);
+
+	/**
+	 * @get_scanout_position:
+	 *
+	 * Called by vblank timestamping code.
+	 *
+	 * Returns the current display scanout position from a CRTC and an
+	 * optional accurate ktime_get() timestamp of when the position was
+	 * measured. Note that this is a helper callback which is only used
+	 * if a driver uses drm_calc_vbltimestamp_from_scanoutpos() for the
+	 * @drm_driver.get_vblank_timestamp callback.
+	 *
+	 * Parameters:
+	 *
+	 * crtc:
+	 *     The CRTC.
+	 * in_vblank_irq:
+	 *     True when called from drm_crtc_handle_vblank(). Some drivers
+	 *     need to apply some workarounds for gpu-specific vblank irq
+	 *     quirks if the flag is set.
+	 * vpos:
+	 *     Target location for current vertical scanout position.
+	 * hpos:
+	 *     Target location for current horizontal scanout position.
+	 * stime:
+	 *     Target location for timestamp taken immediately before
+	 *     scanout position query. Can be NULL to skip timestamp.
+	 * etime:
+	 *     Target location for timestamp taken immediately after
+	 *     scanout position query. Can be NULL to skip timestamp.
+	 * mode:
+	 *     Current display timings.
+	 *
+	 * Returns vpos as a positive number while in active scanout area.
+	 * Returns vpos as a negative number inside vblank, counting the number
+	 * of scanlines to go until end of vblank, e.g., -1 means "one scanline
+	 * until start of active scanout / end of vblank."
+	 *
+	 * Returns:
+	 *
+	 * True on success, false if a reliable scanout position counter could
+	 * not be read out.
+	 */
+	bool (*get_scanout_position)(struct drm_crtc *crtc,
+				     bool in_vblank_irq, int *vpos, int *hpos,
+				     ktime_t *stime, ktime_t *etime,
+				     const struct drm_display_mode *mode);
 };
 
 /**