Also unify/merge with the existing stuff.

I was a bit torn where to put this, but in the end I decided to put all the ioctl/sysfs/debugfs stuff into drm-uapi.rst. That means we have a bit a split with the other uapi related stuff used internally, like drm_file.[hc], but I think overall this makes more sense.

If it's too confusing we can always add more cross-links to make it more discoverable. But the auto-sprinkling of links kernel-doc already does seems sufficient.

Also for prettier docs and more cross-links, switch the internal defines over to an enum, as usual.

v2: Update kerneldoc fro drm_compat_ioctl too (caught by 0day), plus a bit more drive-by polish.

v3: Fix typo, spotted by xerpi on irc (Sergi).

Cc: Sergi Granell xerpi.g.12@gmail.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- Documentation/gpu/drm-internals.rst | 50 ---------------- Documentation/gpu/drm-uapi.rst | 14 +++++ drivers/gpu/drm/drm_ioc32.c | 76 ++++++++++++----------- drivers/gpu/drm/drm_ioctl.c | 50 +++++++++++++++- include/drm/drm_ioctl.h | 116 +++++++++++++++++++++++++++++++----- 5 files changed, 203 insertions(+), 103 deletions(-)

diff --git a/Documentation/gpu/drm-internals.rst b/Documentation/gpu/drm-internals.rst index a09c721f9e89..babfb6143bd9 100644 --- a/Documentation/gpu/drm-internals.rst +++ b/Documentation/gpu/drm-internals.rst @@ -255,56 +255,6 @@ File Operations .. kernel-doc:: drivers/gpu/drm/drm_file.c :export:

-IOCTLs ------- - -struct drm_ioctl_desc *ioctls; int num_ioctls; - Driver-specific ioctls descriptors table. - -Driver-specific ioctls numbers start at DRM_COMMAND_BASE. The ioctls -descriptors table is indexed by the ioctl number offset from the base -value. Drivers can use the DRM_IOCTL_DEF_DRV() macro to initialize -the table entries. - -:: - - DRM_IOCTL_DEF_DRV(ioctl, func, flags) - -``ioctl`` is the ioctl name. Drivers must define the DRM_##ioctl and -DRM_IOCTL_##ioctl macros to the ioctl number offset from -DRM_COMMAND_BASE and the ioctl number respectively. The first macro is -private to the device while the second must be exposed to userspace in a -public header. - -``func`` is a pointer to the ioctl handler function compatible with the -``drm_ioctl_t`` type. - -:: - - typedef int drm_ioctl_t(struct drm_device *dev, void *data, - struct drm_file *file_priv); - -``flags`` is a bitmask combination of the following values. It restricts -how the ioctl is allowed to be called. - -- DRM_AUTH - Only authenticated callers allowed - -- DRM_MASTER - The ioctl can only be called on the master file handle - -- DRM_ROOT_ONLY - Only callers with the SYSADMIN capability allowed - -- DRM_CONTROL_ALLOW - The ioctl can only be called on a control - device - -- DRM_UNLOCKED - The ioctl handler will be called without locking the - DRM global mutex. This is the enforced default for kms drivers (i.e. - using the DRIVER_MODESET flag) and hence shouldn't be used any more - for new drivers. - -.. kernel-doc:: drivers/gpu/drm/drm_ioctl.c - :export: - - Misc Utilities ==============

diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst index 8b0f0e403f0c..858457567d3d 100644 --- a/Documentation/gpu/drm-uapi.rst +++ b/Documentation/gpu/drm-uapi.rst @@ -160,6 +160,20 @@ other hand, a driver requires shared state between clients which is visible to user-space and accessible beyond open-file boundaries, they cannot support render nodes.

+IOCTL Support on Device Nodes +============================= + +.. kernel-doc:: drivers/gpu/drm/drm_ioctl.c + :doc: driver specific ioctls + +.. kernel-doc:: include/drm/drm_ioctl.h + :internal: + +.. kernel-doc:: drivers/gpu/drm/drm_ioctl.c + :export: + +.. kernel-doc:: drivers/gpu/drm/drm_ioc32.c + :export:

Testing and validation ====================== diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c index b134482f4022..0fa7827cb0fd 100644 --- a/drivers/gpu/drm/drm_ioc32.c +++ b/drivers/gpu/drm/drm_ioc32.c @@ -1,4 +1,4 @@ -/** +/* * \file drm_ioc32.c * * 32-bit ioctl compatibility routines for the DRM. @@ -72,15 +72,15 @@ #define DRM_IOCTL_MODE_ADDFB232 DRM_IOWR(0xb8, drm_mode_fb_cmd232_t)

typedef struct drm_version_32 { - int version_major; /**< Major version */ - int version_minor; /**< Minor version */ - int version_patchlevel; /**< Patch level */ - u32 name_len; /**< Length of name buffer */ - u32 name; /**< Name of driver */ - u32 date_len; /**< Length of date buffer */ - u32 date; /**< User-space buffer to hold date */ - u32 desc_len; /**< Length of desc buffer */ - u32 desc; /**< User-space buffer to hold desc */ + int version_major; /* Major version */ + int version_minor; /* Minor version */ + int version_patchlevel; /* Patch level */ + u32 name_len; /* Length of name buffer */ + u32 name; /* Name of driver */ + u32 date_len; /* Length of date buffer */ + u32 date; /* User-space buffer to hold date */ + u32 desc_len; /* Length of desc buffer */ + u32 desc; /* User-space buffer to hold desc */ } drm_version32_t;

static int compat_drm_version(struct file *file, unsigned int cmd, @@ -126,8 +126,8 @@ static int compat_drm_version(struct file *file, unsigned int cmd, }

typedef struct drm_unique32 { - u32 unique_len; /**< Length of unique */ - u32 unique; /**< Unique name for driver instantiation */ + u32 unique_len; /* Length of unique */ + u32 unique; /* Unique name for driver instantiation */ } drm_unique32_t;

static int compat_drm_getunique(struct file *file, unsigned int cmd, @@ -180,12 +180,12 @@ static int compat_drm_setunique(struct file *file, unsigned int cmd, }

typedef struct drm_map32 { - u32 offset; /**< Requested physical address (0 for SAREA)*/ - u32 size; /**< Requested physical size (bytes) */ - enum drm_map_type type; /**< Type of memory to map */ - enum drm_map_flags flags; /**< Flags */ - u32 handle; /**< User-space: "Handle" to pass to mmap() */ - int mtrr; /**< MTRR slot used */ + u32 offset; /* Requested physical address (0 for SAREA)*/ + u32 size; /* Requested physical size (bytes) */ + enum drm_map_type type; /* Type of memory to map */ + enum drm_map_flags flags; /* Flags */ + u32 handle; /* User-space: "Handle" to pass to mmap() */ + int mtrr; /* MTRR slot used */ } drm_map32_t;

static int compat_drm_getmap(struct file *file, unsigned int cmd, @@ -286,12 +286,12 @@ static int compat_drm_rmmap(struct file *file, unsigned int cmd, }

typedef struct drm_client32 { - int idx; /**< Which client desired? */ - int auth; /**< Is client authenticated? */ - u32 pid; /**< Process ID */ - u32 uid; /**< User ID */ - u32 magic; /**< Magic */ - u32 iocs; /**< Ioctl count */ + int idx; /* Which client desired? */ + int auth; /* Is client authenticated? */ + u32 pid; /* Process ID */ + u32 uid; /* User ID */ + u32 magic; /* Magic */ + u32 iocs; /* Ioctl count */ } drm_client32_t;

static int compat_drm_getclient(struct file *file, unsigned int cmd, @@ -366,12 +366,12 @@ static int compat_drm_getstats(struct file *file, unsigned int cmd, }

typedef struct drm_buf_desc32 { - int count; /**< Number of buffers of this size */ - int size; /**< Size in bytes */ - int low_mark; /**< Low water mark */ - int high_mark; /**< High water mark */ + int count; /* Number of buffers of this size */ + int size; /* Size in bytes */ + int low_mark; /* Low water mark */ + int high_mark; /* High water mark */ int flags; - u32 agp_start; /**< Start address in the AGP aperture */ + u32 agp_start; /* Start address in the AGP aperture */ } drm_buf_desc32_t;

static int compat_drm_addbufs(struct file *file, unsigned int cmd, @@ -1111,13 +1111,18 @@ static drm_ioctl_compat_t *drm_compat_ioctls[] = { };

/** - * Called whenever a 32-bit process running under a 64-bit kernel - * performs an ioctl on /dev/drm. + * drm_compat_ioctl - 32bit IOCTL compatibility handler for DRM drivers + * @filp: file this ioctl is called on + * @cmd: ioctl cmd number + * @arg: user argument + * + * Compatibility handler for 32 bit userspace running on 64 kernels. All actual + * IOCTL handling is forwarded to drm_ioctl(), while marshalling structures as + * appropriate. Note that this only handles DRM core IOCTLs, if the driver has + * botched IOCTL itself, it must handle those by wrapping this function. * - * \param file_priv DRM file private. - * \param cmd command. - * \param arg user argument. - * \return zero on success or negative number on failure. + * Returns: + * Zero on success, negative error code on failure. */ long drm_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) { @@ -1141,5 +1146,4 @@ long drm_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)

return ret; } - EXPORT_SYMBOL(drm_compat_ioctl); diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 7f4f4f48e390..f908ea94b784 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -647,13 +647,59 @@ static const struct drm_ioctl_desc drm_ioctls[] = { #define DRM_CORE_IOCTL_COUNT ARRAY_SIZE( drm_ioctls )

/** + * DOC: driver specific ioctls + * + * First things first, driver private IOCTLs should only be needed for drivers + * supporting rendering. Kernel modesetting is all standardized, and extended + * through properties. There are a few exceptions in some existing drivers, + * which define IOCTL for use by the display DRM master, but they all predate + * properties. + * + * Now if you do have a render driver you always have to support it through + * driver private properties. There's a few steps needed to wire all the things + * up. + * + * First you need to define the structure for your IOCTL in your driver private + * UAPI header in ``include/uapi/drm/my_driver_drm.h``:: + * + * struct my_driver_operation { + * u32 some_thing; + * u32 another_thing; + * }; + * + * Please make sure that you follow all the best practices from + * ``Documentation/ioctl/botching-up-ioctls.txt``. Note that drm_ioctl() + * automatically zero-extends structures, hence make sure you can add more stuff + * at the end, i.e. don't put a variable sized array there. + * + * Then you need to define your IOCTL number, using one of DRM_IO(), DRM_IOR(), + * DRM_IOW() or DRM_IOWR(). It must start with the DRM_IOCTL_ prefix:: + * + * ##define DRM_IOCTL_MY_DRIVER_OPERATION \ + * DRM_IOW(DRM_COMMAND_BASE, struct my_driver_operation) + * + * DRM driver private IOCTL must be in the range from DRM_COMMAND_BASE to + * DRM_COMMAND_END. Finally you need an array of &struct drm_ioctl_desc to wire + * up the handlers and set the access rights: + * + * static const struct drm_ioctl_desc my_driver_ioctls[] = { + * DRM_IOCTL_DEF_DRV(MY_DRIVER_OPERATION, my_driver_operation, + * DRM_AUTH|DRM_RENDER_ALLOW), + * }; + * + * And then assign this to the &drm_driver.ioctls field in your driver + * structure. + */ + +/** * drm_ioctl - ioctl callback implementation for DRM drivers * @filp: file this ioctl is called on * @cmd: ioctl cmd number * @arg: user argument * - * Looks up the ioctl function in the ::ioctls table, checking for root - * previleges if so required, and dispatches to the respective function. + * Looks up the ioctl function in the DRM core and the driver dispatch table, + * stored in &drm_driver.ioctls. It checks for necessary permission by calling + * drm_ioctl_permit(), and dispatches to the respective function. * * Returns: * Zero on success, negative error code on failure. diff --git a/include/drm/drm_ioctl.h b/include/drm/drm_ioctl.h index f17ee077f649..ee03b3c44b3b 100644 --- a/include/drm/drm_ioctl.h +++ b/include/drm/drm_ioctl.h @@ -33,6 +33,7 @@ #define _DRM_IOCTL_H_

#include <linux/types.h> +#include <linux/bitops.h>

#include <asm/ioctl.h>

@@ -41,41 +42,126 @@ struct drm_file; struct file;

/** - * Ioctl function type. + * drm_ioctl_t - DRM ioctl function type. + * @dev: DRM device inode + * @data: private pointer of the ioctl call + * @file_priv: DRM file this ioctl was made on * - * \param inode device inode. - * \param file_priv DRM file private pointer. - * \param cmd command. - * \param arg argument. + * This is the DRM ioctl typedef. Note that drm_ioctl() has alrady copied @data + * into kernel-space, and will also copy it back, depending upon the read/write + * settings in the ioctl command code. */ typedef int drm_ioctl_t(struct drm_device *dev, void *data, struct drm_file *file_priv);

+/** + * drm_ioctl_compat_t - compatibility DRM ioctl function type. + * @filp: file pointer + * @cmd: ioctl command code + * @arg: DRM file this ioctl was made on + * + * Just a typedef to make declaring an array of compatibility handlers easier. + * New drivers shouldn't screw up the structure layout for their ioctl + * structures and hence never need this. + */ typedef int drm_ioctl_compat_t(struct file *filp, unsigned int cmd, unsigned long arg);

#define DRM_IOCTL_NR(n) _IOC_NR(n) #define DRM_MAJOR 226

-#define DRM_AUTH 0x1 -#define DRM_MASTER 0x2 -#define DRM_ROOT_ONLY 0x4 -#define DRM_CONTROL_ALLOW 0x8 -#define DRM_UNLOCKED 0x10 -#define DRM_RENDER_ALLOW 0x20 +/** + * enum drm_ioctl_flags - DRM ioctl flags + * + * Various flags that can be set in &drm_ioctl_desc.flags to control how + * userspace can use a given ioctl. + */ +enum drm_ioctl_flags { + /** + * @DRM_AUTH: + * + * This is for ioctl which are used for rendering, and require that the + * file descriptor is either for a render node, or if it's a + * legacy/primary node, then it must be authenticated. + */ + DRM_AUTH = BIT(0), + /** + * @DRM_MASTER: + * + * This must be set for any ioctl which can change the modeset or + * display state. Userspace must call the ioctl through a primary node, + * while it is the active master. + * + * Note that read-only modeset ioctl can also be called by + * unauthenticated clients, or when a master is not the currently active + * one. + */ + DRM_MASTER = BIT(1), + /** + * @DRM_ROOT_ONLY: + * + * Anything that could potentially wreak a master file descriptor needs + * to have this flag set. Current that's only for the SETMASTER and + * DROPMASTER ioctl, which e.g. logind can call to force a non-behaving + * master (display compositor) into compliance. + * + * This is equivalent to callers with the SYSADMIN capability. + */ + DRM_ROOT_ONLY = BIT(2), + /** + * @DRM_CONTROL_ALLOW: + * + * Deprecated, do not use. Control nodes are in the process of getting + * removed. + */ + DRM_CONTROL_ALLOW = BIT(3), + /** + * @DRM_UNLOCKED: + * + * Whether &drm_ioctl_desc.func should be called with the DRM BKL held + * or not. Enforced as the default for all modern drivers, hence there + * should never be a need to set this flag. + */ + DRM_UNLOCKED = BIT(4), + /** + * @DRM_RENDER_ALLOW: + * + * This is used for all ioctl needed for rendering only, for drivers + * which support render nodes. This should be all new render drivers, + * and hence it should be always set for any ioctl with DRM_AUTH set. + * Note though that read-only query ioctl might have this set, but have + * not set DRM_AUTH because they do not require authentication. + */ + DRM_RENDER_ALLOW = BIT(5), +};

+/** + * struct drm_ioctl_desc - DRM driver ioctl entry + * @cmd: ioctl command number, without flags + * @flags: a bitmask of &enum drm_ioctl_flags + * @func: handler for this ioctl + * @name: user-readable name for debug output + * + * For convenience it's easier to create these using the DRM_IOCTL_DEF_DRV() + * macro. + */ struct drm_ioctl_desc { unsigned int cmd; - int flags; + enum drm_ioctl_flags flags; drm_ioctl_t *func; const char *name; };

/** - * Creates a driver or general drm_ioctl_desc array entry for the given - * ioctl, for use by drm_ioctl(). + * DRM_IOCTL_DEF_DRV() - helper macro to fill out a &struct drm_ioctl_desc + * @ioctl: ioctl command suffix + * @_func: handler for the ioctl + * @_flags: a bitmask of &enum drm_ioctl_flags + * + * Small helper macro to create a &struct drm_ioctl_desc entry. The ioctl + * command number is constructed by prepending ``DRM_IOCTL_`` and passing that + * to DRM_IOCTL_NR(). */ - #define DRM_IOCTL_DEF_DRV(ioctl, _func, _flags) \ [DRM_IOCTL_NR(DRM_IOCTL_##ioctl) - DRM_COMMAND_BASE] = { \ .cmd = DRM_IOCTL_##ioctl, \

There's really no reason for anything more: - Calling this while the crtc vblank stuff isn't set up is a driver bug. Those places arlready DRM_ERROR. - Calling this when the crtc is off is either a driver bug (callin drm_crtc_handle_vblank at the wrong time) or a core bug (for anything else). Again, we DRM_ERROR. - EINVAL is checked at higher levels already, and if we'd use struct drm_crtc * instead of (dev, pipe) it would be real obvious that those are again core bugs.

The only valid failure mode is crap hardware that couldn't sample a useful timestamp, to ask the core to just grab a not-so-accurate timestampe. Bool is perfectly fine for that.

v2: Also fix up the one caller, I lost that in the shuffling (Jani).

Cc: Jani Nikula jani.nikula@linux.intel.com Cc: Mario Kleiner mario.kleiner@tuebingen.mpg.de Cc: Eric Anholt eric@anholt.net Cc: Rob Clark robdclark@gmail.com Cc: linux-arm-msm@vger.kernel.org Cc: freedreno@lists.freedesktop.org Cc: Alex Deucher alexander.deucher@amd.com Cc: Christian König christian.koenig@amd.com Cc: Ben Skeggs bskeggs@redhat.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 8 ++--- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 14 ++++----- drivers/gpu/drm/drm_irq.c | 49 ++++++++++++------------------- drivers/gpu/drm/i915/i915_irq.c | 8 ++--- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 12 ++++---- drivers/gpu/drm/nouveau/nouveau_display.c | 4 +-- drivers/gpu/drm/nouveau/nouveau_display.h | 4 +-- drivers/gpu/drm/radeon/radeon_drv.c | 8 ++--- drivers/gpu/drm/radeon/radeon_kms.c | 14 ++++----- drivers/gpu/drm/vc4/vc4_crtc.c | 2 +- drivers/gpu/drm/vc4/vc4_drv.h | 2 +- include/drm/drmP.h | 1 - include/drm/drm_drv.h | 7 ++--- include/drm/drm_irq.h | 10 +++---- 14 files changed, 64 insertions(+), 79 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 262056778f52..0dcc9ee73ab1 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -1913,10 +1913,10 @@ int amdgpu_device_resume(struct drm_device *dev, bool resume, bool fbcon); u32 amdgpu_get_vblank_counter_kms(struct drm_device *dev, unsigned int pipe); int amdgpu_enable_vblank_kms(struct drm_device *dev, unsigned int pipe); void amdgpu_disable_vblank_kms(struct drm_device *dev, unsigned int pipe); -int amdgpu_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe, - int *max_error, - struct timeval *vblank_time, - unsigned flags); +bool amdgpu_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe, + int *max_error, + struct timeval *vblank_time, + unsigned flags); long amdgpu_kms_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c index dfb029ab3448..ba937d763df5 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c @@ -951,19 +951,19 @@ void amdgpu_disable_vblank_kms(struct drm_device *dev, unsigned int pipe) * * Gets the timestamp on the requested crtc based on the * scanout position. (all asics). - * Returns postive status flags on success, negative error on failure. + * Returns true on success, false on failure. */ -int amdgpu_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe, - int *max_error, - struct timeval *vblank_time, - unsigned flags) +bool amdgpu_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe, + int *max_error, + struct timeval *vblank_time, + unsigned flags) { struct drm_crtc *crtc; struct amdgpu_device *adev = dev->dev_private;

if (pipe >= dev->num_crtcs) { DRM_ERROR("Invalid crtc %u\n", pipe); - return -EINVAL; + return false; }

/* Get associated drm_crtc: */ @@ -972,7 +972,7 @@ int amdgpu_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe, /* This can occur on driver load if some component fails to * initialize completely and driver is unloaded */ DRM_ERROR("Uninitialized crtc %d\n", pipe); - return -EINVAL; + return false; }

/* Helper routine in DRM core does all the work: */ diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index dac1b2593cb1..319ab3661965 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -724,43 +724,32 @@ EXPORT_SYMBOL(drm_calc_timestamping_constants); * active. Higher level code is expected to handle this. * * Returns: - * Negative value on error, failure or if not supported in current - * video mode: - * - * -EINVAL Invalid CRTC. - * -EAGAIN Temporary unavailable, e.g., called before initial modeset. - * -ENOTSUPP Function not supported in current display mode. - * -EIO Failed, e.g., due to failed scanout position query. - * - * Returns or'ed positive status flags on success: - * - * DRM_VBLANKTIME_SCANOUTPOS_METHOD - Signal this method used for timestamping. - * DRM_VBLANKTIME_INVBL - Timestamp taken while scanout was in vblank interval. * + * Returns true on success, and false on failure, i.e. when no accurate + * timestamp could be acquired. */ -int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, - unsigned int pipe, - int *max_error, - struct timeval *vblank_time, - unsigned flags, - const struct drm_display_mode *mode) +bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, + unsigned int pipe, + int *max_error, + struct timeval *vblank_time, + unsigned flags, + const struct drm_display_mode *mode) { struct timeval tv_etime; ktime_t stime, etime; unsigned int vbl_status; - int ret = DRM_VBLANKTIME_SCANOUTPOS_METHOD; int vpos, hpos, i; int delta_ns, duration_ns;

if (pipe >= dev->num_crtcs) { DRM_ERROR("Invalid crtc %u\n", pipe); - return -EINVAL; + return false; }

/* 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"); - return -EIO; + return false; }

/* If mode timing undefined, just return as no-op: @@ -768,7 +757,7 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, */ if (mode->crtc_clock == 0) { DRM_DEBUG("crtc %u: Noop due to uninitialized mode.\n", pipe); - return -EAGAIN; + return false; }

/* Get current scanout position with system timestamp. @@ -792,7 +781,7 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, if (!(vbl_status & DRM_SCANOUTPOS_VALID)) { DRM_DEBUG("crtc %u : scanoutpos query failed [0x%x].\n", pipe, vbl_status); - return -EIO; + return false; }

/* Compute uncertainty in timestamp of scanout position query. */ @@ -836,7 +825,7 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, (long)vblank_time->tv_sec, (long)vblank_time->tv_usec, duration_ns/1000, i);

- return ret; + return true; } EXPORT_SYMBOL(drm_calc_vbltimestamp_from_scanoutpos);

@@ -872,25 +861,23 @@ static bool drm_get_last_vbltimestamp(struct drm_device *dev, unsigned int pipe, struct timeval *tvblank, unsigned flags) { - int ret; + bool ret = false;

/* Define requested maximum error on timestamps (nanoseconds). */ int max_error = (int) drm_timestamp_precision * 1000;

/* Query driver if possible and precision timestamping enabled. */ - if (dev->driver->get_vblank_timestamp && (max_error > 0)) { + if (dev->driver->get_vblank_timestamp && (max_error > 0)) ret = dev->driver->get_vblank_timestamp(dev, pipe, &max_error, tvblank, flags); - if (ret > 0) - return true; - }

/* GPU high precision timestamp query unsupported or failed. * Return current monotonic/gettimeofday timestamp as best estimate. */ - *tvblank = get_drm_timestamp(); + if (!ret) + *tvblank = get_drm_timestamp();

- return false; + return ret; }

/** diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 30daa809bd72..7595b9a7b5f3 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -964,7 +964,7 @@ int intel_get_crtc_scanline(struct intel_crtc *crtc) return position; }

-static int i915_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe, +static bool i915_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe, int *max_error, struct timeval *vblank_time, unsigned flags) @@ -974,19 +974,19 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe,

if (pipe >= INTEL_INFO(dev_priv)->num_pipes) { DRM_ERROR("Invalid crtc %u\n", pipe); - return -EINVAL; + return false; }

/* Get drm_crtc to timestamp: */ crtc = intel_get_crtc_for_pipe(dev_priv, pipe); if (crtc == NULL) { DRM_ERROR("Invalid crtc %u\n", pipe); - return -EINVAL; + return false; }

if (!crtc->base.hwmode.crtc_clock) { DRM_DEBUG_KMS("crtc %u is disabled\n", pipe); - return -EBUSY; + return false; }

/* Helper routine in DRM core does all the work: */ diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c index 41ccd2a15d3c..655700eb42ba 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c @@ -595,23 +595,23 @@ static int mdp5_get_scanoutpos(struct drm_device *dev, unsigned int pipe, return ret; }

-static int mdp5_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe, - int *max_error, - struct timeval *vblank_time, - unsigned flags) +static bool mdp5_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe, + int *max_error, + struct timeval *vblank_time, + unsigned flags) { struct msm_drm_private *priv = dev->dev_private; struct drm_crtc *crtc;

if (pipe < 0 || pipe >= priv->num_crtcs) { DRM_ERROR("Invalid crtc %d\n", pipe); - return -EINVAL; + return false; }

crtc = priv->crtcs[pipe]; if (!crtc) { DRM_ERROR("Invalid crtc %d\n", pipe); - return -EINVAL; + return false; }

return drm_calc_vbltimestamp_from_scanoutpos(dev, pipe, max_error, diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c index 6104f61b00fc..9375f41da523 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.c +++ b/drivers/gpu/drm/nouveau/nouveau_display.c @@ -156,7 +156,7 @@ nouveau_display_scanoutpos(struct drm_device *dev, unsigned int pipe, return 0; }

-int +bool nouveau_display_vblstamp(struct drm_device *dev, unsigned int pipe, int *max_error, struct timeval *time, unsigned flags) { @@ -174,7 +174,7 @@ nouveau_display_vblstamp(struct drm_device *dev, unsigned int pipe, } }

- return -EINVAL; + return false; }

static void diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h b/drivers/gpu/drm/nouveau/nouveau_display.h index e1d772d39488..c6bfe533a641 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.h +++ b/drivers/gpu/drm/nouveau/nouveau_display.h @@ -71,8 +71,8 @@ void nouveau_display_vblank_disable(struct drm_device *, unsigned int); int nouveau_display_scanoutpos(struct drm_device *, unsigned int, unsigned int, int *, int *, ktime_t *, ktime_t *, const struct drm_display_mode *); -int nouveau_display_vblstamp(struct drm_device *, unsigned int, int *, - struct timeval *, unsigned); +bool nouveau_display_vblstamp(struct drm_device *, unsigned int, int *, + struct timeval *, unsigned);

int nouveau_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb, struct drm_pending_vblank_event *event, diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c index 93d45aa5c3d4..caa0b1cc4269 100644 --- a/drivers/gpu/drm/radeon/radeon_drv.c +++ b/drivers/gpu/drm/radeon/radeon_drv.c @@ -115,10 +115,10 @@ int radeon_resume_kms(struct drm_device *dev, bool resume, bool fbcon); u32 radeon_get_vblank_counter_kms(struct drm_device *dev, unsigned int pipe); int radeon_enable_vblank_kms(struct drm_device *dev, unsigned int pipe); void radeon_disable_vblank_kms(struct drm_device *dev, unsigned int pipe); -int radeon_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe, - int *max_error, - struct timeval *vblank_time, - unsigned flags); +bool radeon_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe, + int *max_error, + struct timeval *vblank_time, + unsigned flags); void radeon_driver_irq_preinstall_kms(struct drm_device *dev); int radeon_driver_irq_postinstall_kms(struct drm_device *dev); void radeon_driver_irq_uninstall_kms(struct drm_device *dev); diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c index e3e7cb1d10a2..535969d74f64 100644 --- a/drivers/gpu/drm/radeon/radeon_kms.c +++ b/drivers/gpu/drm/radeon/radeon_kms.c @@ -869,25 +869,25 @@ void radeon_disable_vblank_kms(struct drm_device *dev, int crtc) * * Gets the timestamp on the requested crtc based on the * scanout position. (all asics). - * Returns postive status flags on success, negative error on failure. + * Returns true on success, false on failure. */ -int radeon_get_vblank_timestamp_kms(struct drm_device *dev, int crtc, - int *max_error, - struct timeval *vblank_time, - unsigned flags) +bool radeon_get_vblank_timestamp_kms(struct drm_device *dev, int crtc, + int *max_error, + struct timeval *vblank_time, + unsigned flags) { struct drm_crtc *drmcrtc; struct radeon_device *rdev = dev->dev_private;

if (crtc < 0 || crtc >= dev->num_crtcs) { DRM_ERROR("Invalid crtc %d\n", crtc); - return -EINVAL; + return false; }

/* Get associated drm_crtc: */ drmcrtc = &rdev->mode_info.crtcs[crtc]->base; if (!drmcrtc) - return -EINVAL; + return false;

/* Helper routine in DRM core does all the work: */ return drm_calc_vbltimestamp_from_scanoutpos(dev, crtc, max_error, diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index e8f28c9145a0..60fefe9d7b8c 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -270,7 +270,7 @@ int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id, return ret; }

-int vc4_crtc_get_vblank_timestamp(struct drm_device *dev, unsigned int crtc_id, +bool vc4_crtc_get_vblank_timestamp(struct drm_device *dev, unsigned int crtc_id, int *max_error, struct timeval *vblank_time, unsigned flags) { diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index dffce6293d87..8a5d0e12ee02 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -450,7 +450,7 @@ int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id, unsigned int flags, int *vpos, int *hpos, ktime_t *stime, ktime_t *etime, const struct drm_display_mode *mode); -int vc4_crtc_get_vblank_timestamp(struct drm_device *dev, unsigned int crtc_id, +bool vc4_crtc_get_vblank_timestamp(struct drm_device *dev, unsigned int crtc_id, int *max_error, struct timeval *vblank_time, unsigned flags);

diff --git a/include/drm/drmP.h b/include/drm/drmP.h index a5ddc2815bf4..1b3f3cd7b230 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -323,7 +323,6 @@ struct pci_controller;

/* Flags and return codes for get_vblank_timestamp() driver function. */ #define DRM_CALLED_FROM_VBLIRQ 1 -#define DRM_VBLANKTIME_SCANOUTPOS_METHOD (1 << 0)

/* get_scanout_position() return flags */ #define DRM_SCANOUTPOS_VALID (1 << 0) diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index d0b5f363bfa1..da78e248d9d8 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -316,11 +316,10 @@ struct drm_driver { * * Returns: * - * Zero if timestamping isn't supported in current display mode or a - * negative number on failure. A positive status code on success, - * which describes how the vblank_time timestamp was computed. + * True on success, false on failure, which means the core should + * fallback to a simple timestamp taken in drm_crtc_handle_vblank(). */ - int (*get_vblank_timestamp) (struct drm_device *dev, unsigned int pipe, + bool (*get_vblank_timestamp) (struct drm_device *dev, unsigned int pipe, int *max_error, struct timeval *vblank_time, unsigned flags); diff --git a/include/drm/drm_irq.h b/include/drm/drm_irq.h index cf0be6594c8c..f0d5ccf9b282 100644 --- a/include/drm/drm_irq.h +++ b/include/drm/drm_irq.h @@ -153,11 +153,11 @@ void drm_crtc_vblank_on(struct drm_crtc *crtc); void drm_vblank_cleanup(struct drm_device *dev); u32 drm_accurate_vblank_count(struct drm_crtc *crtc);

-int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, - unsigned int pipe, int *max_error, - struct timeval *vblank_time, - unsigned flags, - const struct drm_display_mode *mode); +bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, + unsigned int pipe, int *max_error, + struct timeval *vblank_time, + unsigned flags, + const struct drm_display_mode *mode); void drm_calc_timestamping_constants(struct drm_crtc *crtc, const struct drm_display_mode *mode);

It's overkill to have a flag parameter which is essentially used just as a boolean. This takes care of core + adjusting drivers.

Adjusting the scanout position callback is a bit harder, since radeon also supplies it's own driver-private flags in there.

Cc: Mario Kleiner mario.kleiner@tuebingen.mpg.de Cc: Eric Anholt eric@anholt.net Cc: Rob Clark robdclark@gmail.com Cc: linux-arm-msm@vger.kernel.org Cc: freedreno@lists.freedesktop.org Cc: Alex Deucher alexander.deucher@amd.com Cc: Christian König christian.koenig@amd.com Cc: Ben Skeggs bskeggs@redhat.com Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 6 ++--- drivers/gpu/drm/drm_irq.c | 41 +++++++++++++++++-------------- drivers/gpu/drm/i915/i915_irq.c | 4 +-- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 4 +-- drivers/gpu/drm/nouveau/nouveau_display.c | 5 ++-- drivers/gpu/drm/nouveau/nouveau_display.h | 2 +- drivers/gpu/drm/radeon/radeon_drv.c | 2 +- drivers/gpu/drm/radeon/radeon_kms.c | 4 +-- drivers/gpu/drm/vc4/vc4_crtc.c | 4 +-- drivers/gpu/drm/vc4/vc4_drv.h | 2 +- include/drm/drm_drv.h | 11 ++++----- include/drm/drm_irq.h | 2 +- 13 files changed, 46 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 0dcc9ee73ab1..310b20ef794a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -1916,7 +1916,7 @@ void amdgpu_disable_vblank_kms(struct drm_device *dev, unsigned int pipe); bool amdgpu_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe, int *max_error, struct timeval *vblank_time, - unsigned flags); + bool in_vblank_irq); long amdgpu_kms_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c index ba937d763df5..109a77ffc692 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c @@ -947,7 +947,7 @@ void amdgpu_disable_vblank_kms(struct drm_device *dev, unsigned int pipe) * @crtc: crtc to get the timestamp for * @max_error: max error * @vblank_time: time value - * @flags: flags passed to the driver + * @in_vblank_irq: called from drm_handle_vblank() * * Gets the timestamp on the requested crtc based on the * scanout position. (all asics). @@ -956,7 +956,7 @@ void amdgpu_disable_vblank_kms(struct drm_device *dev, unsigned int pipe) bool amdgpu_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe, int *max_error, struct timeval *vblank_time, - unsigned flags) + bool in_vblank_irq) { struct drm_crtc *crtc; struct amdgpu_device *adev = dev->dev_private; @@ -977,7 +977,7 @@ bool amdgpu_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe,

/* Helper routine in DRM core does all the work: */ return drm_calc_vbltimestamp_from_scanoutpos(dev, pipe, max_error, - vblank_time, flags, + vblank_time, in_vblank_irq, &crtc->hwmode); }

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 319ab3661965..82b6e0521fe1 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -54,7 +54,7 @@

static bool drm_get_last_vbltimestamp(struct drm_device *dev, unsigned int pipe, - struct timeval *tvblank, unsigned flags); + struct timeval *tvblank, bool in_vblank_irq);

static unsigned int drm_timestamp_precision = 20; /* Default to 20 usecs. */

@@ -138,7 +138,7 @@ static void drm_reset_vblank_timestamp(struct drm_device *dev, unsigned int pipe */ do { cur_vblank = __get_vblank_counter(dev, pipe); - rc = drm_get_last_vbltimestamp(dev, pipe, &t_vblank, 0); + rc = drm_get_last_vbltimestamp(dev, pipe, &t_vblank, false); } while (cur_vblank != __get_vblank_counter(dev, pipe) && --count > 0);

/* @@ -171,7 +171,7 @@ static void drm_reset_vblank_timestamp(struct drm_device *dev, unsigned int pipe * device vblank fields. */ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, - unsigned long flags) + bool in_vblank_irq) { struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; u32 cur_vblank, diff; @@ -194,7 +194,7 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, */ do { cur_vblank = __get_vblank_counter(dev, pipe); - rc = drm_get_last_vbltimestamp(dev, pipe, &t_vblank, flags); + rc = drm_get_last_vbltimestamp(dev, pipe, &t_vblank, in_vblank_irq); } while (cur_vblank != __get_vblank_counter(dev, pipe) && --count > 0);

if (dev->max_vblank_count != 0) { @@ -214,13 +214,13 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, */ diff = DIV_ROUND_CLOSEST_ULL(diff_ns, framedur_ns);

- if (diff == 0 && flags & DRM_CALLED_FROM_VBLIRQ) + if (diff == 0 && in_vblank_irq) DRM_DEBUG_VBL("crtc %u: Redundant vblirq ignored." " diff_ns = %lld, framedur_ns = %d)\n", pipe, (long long) diff_ns, framedur_ns); } else { /* some kind of default for drivers w/o accurate vbl timestamping */ - diff = (flags & DRM_CALLED_FROM_VBLIRQ) != 0; + diff = in_vblank_irq ? 1 : 0; }

/* @@ -253,7 +253,7 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, * Otherwise reinitialize delayed at next vblank interrupt and assign 0 * for now, to mark the vblanktimestamp as invalid. */ - if (!rc && (flags & DRM_CALLED_FROM_VBLIRQ) == 0) + if (!rc && in_vblank_irq) t_vblank = (struct timeval) {0, 0};

store_vblank(dev, pipe, diff, &t_vblank, cur_vblank); @@ -291,7 +291,7 @@ u32 drm_accurate_vblank_count(struct drm_crtc *crtc)

spin_lock_irqsave(&dev->vblank_time_lock, flags);

- drm_update_vblank_count(dev, pipe, 0); + drm_update_vblank_count(dev, pipe, false); vblank = drm_vblank_count(dev, pipe);

spin_unlock_irqrestore(&dev->vblank_time_lock, flags); @@ -349,7 +349,7 @@ static void vblank_disable_and_save(struct drm_device *dev, unsigned int pipe) * this time. This makes the count account for the entire time * between drm_crtc_vblank_on() and drm_crtc_vblank_off(). */ - drm_update_vblank_count(dev, pipe, 0); + drm_update_vblank_count(dev, pipe, false);

spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags); } @@ -700,9 +700,10 @@ EXPORT_SYMBOL(drm_calc_timestamping_constants); * @max_error: Desired maximum allowable error in timestamps (nanosecs) * On return contains true maximum error of timestamp * @vblank_time: Pointer to struct timeval which should receive the timestamp - * @flags: Flags to pass to driver: - * 0 = Default, - * DRM_CALLED_FROM_VBLIRQ = If function is called from vbl IRQ handler + * @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 flag is set. * @mode: mode which defines the scanout timings * * Implements calculation of exact vblank timestamps from given drm_display_mode @@ -732,7 +733,7 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, unsigned int pipe, int *max_error, struct timeval *vblank_time, - unsigned flags, + bool in_vblank_irq, const struct drm_display_mode *mode) { struct timeval tv_etime; @@ -740,6 +741,7 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, unsigned int vbl_status; int vpos, hpos, i; int delta_ns, duration_ns; + unsigned flags = in_vblank_irq ? DRM_CALLED_FROM_VBLIRQ : 0;

if (pipe >= dev->num_crtcs) { DRM_ERROR("Invalid crtc %u\n", pipe); @@ -843,9 +845,10 @@ static struct timeval get_drm_timestamp(void) * @dev: DRM device * @pipe: index of CRTC whose vblank timestamp to retrieve * @tvblank: Pointer to target struct timeval which should receive the timestamp - * @flags: Flags to pass to driver: - * 0 = Default, - * DRM_CALLED_FROM_VBLIRQ = If function is called from vbl IRQ handler + * @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 flag is set. * * Fetches the system timestamp corresponding to the time of the most recent * vblank interval on specified CRTC. May call into kms-driver to @@ -859,7 +862,7 @@ static struct timeval get_drm_timestamp(void) */ static bool drm_get_last_vbltimestamp(struct drm_device *dev, unsigned int pipe, - struct timeval *tvblank, unsigned flags) + struct timeval *tvblank, bool in_vblank_irq) { bool ret = false;

@@ -869,7 +872,7 @@ drm_get_last_vbltimestamp(struct drm_device *dev, unsigned int pipe, /* Query driver if possible and precision timestamping enabled. */ if (dev->driver->get_vblank_timestamp && (max_error > 0)) ret = dev->driver->get_vblank_timestamp(dev, pipe, &max_error, - tvblank, flags); + tvblank, in_vblank_irq);

/* GPU high precision timestamp query unsupported or failed. * Return current monotonic/gettimeofday timestamp as best estimate. @@ -1745,7 +1748,7 @@ bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe) return false; }

- drm_update_vblank_count(dev, pipe, DRM_CALLED_FROM_VBLIRQ); + drm_update_vblank_count(dev, pipe, true);

spin_unlock(&dev->vblank_time_lock);

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 7595b9a7b5f3..6e1063d3629e 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -967,7 +967,7 @@ int intel_get_crtc_scanline(struct intel_crtc *crtc) static bool i915_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe, int *max_error, struct timeval *vblank_time, - unsigned flags) + bool in_vblank_irq) { struct drm_i915_private *dev_priv = to_i915(dev); struct intel_crtc *crtc; @@ -991,7 +991,7 @@ static bool i915_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe,

/* Helper routine in DRM core does all the work: */ return drm_calc_vbltimestamp_from_scanoutpos(dev, pipe, max_error, - vblank_time, flags, + vblank_time, in_vblank_irq, &crtc->base.hwmode); }

diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c index 655700eb42ba..16184ccbdd3b 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c @@ -598,7 +598,7 @@ static int mdp5_get_scanoutpos(struct drm_device *dev, unsigned int pipe, static bool mdp5_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe, int *max_error, struct timeval *vblank_time, - unsigned flags) + bool in_vblank_irq) { struct msm_drm_private *priv = dev->dev_private; struct drm_crtc *crtc; @@ -615,7 +615,7 @@ static bool mdp5_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe, }

return drm_calc_vbltimestamp_from_scanoutpos(dev, pipe, max_error, - vblank_time, flags, + vblank_time, in_vblank_irq, &crtc->mode); }

diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c index 9375f41da523..c7caa0674c54 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.c +++ b/drivers/gpu/drm/nouveau/nouveau_display.c @@ -158,7 +158,7 @@ nouveau_display_scanoutpos(struct drm_device *dev, unsigned int pipe,

bool nouveau_display_vblstamp(struct drm_device *dev, unsigned int pipe, - int *max_error, struct timeval *time, unsigned flags) + int *max_error, struct timeval *time, bool in_vblank_irq) { struct drm_crtc *crtc;

@@ -170,7 +170,8 @@ nouveau_display_vblstamp(struct drm_device *dev, unsigned int pipe, else mode = &crtc->hwmode; return drm_calc_vbltimestamp_from_scanoutpos(dev, - pipe, max_error, time, flags, mode); + pipe, max_error, time, in_vblank_irq, + mode); } }

diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h b/drivers/gpu/drm/nouveau/nouveau_display.h index c6bfe533a641..8ec86259c5ac 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.h +++ b/drivers/gpu/drm/nouveau/nouveau_display.h @@ -72,7 +72,7 @@ int nouveau_display_scanoutpos(struct drm_device *, unsigned int, unsigned int, int *, int *, ktime_t *, ktime_t *, const struct drm_display_mode *); bool nouveau_display_vblstamp(struct drm_device *, unsigned int, int *, - struct timeval *, unsigned); + struct timeval *, bool);

int nouveau_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb, struct drm_pending_vblank_event *event, diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c index caa0b1cc4269..88fc791ec8fb 100644 --- a/drivers/gpu/drm/radeon/radeon_drv.c +++ b/drivers/gpu/drm/radeon/radeon_drv.c @@ -118,7 +118,7 @@ void radeon_disable_vblank_kms(struct drm_device *dev, unsigned int pipe); bool radeon_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe, int *max_error, struct timeval *vblank_time, - unsigned flags); + bool in_vblank_irq); void radeon_driver_irq_preinstall_kms(struct drm_device *dev); int radeon_driver_irq_postinstall_kms(struct drm_device *dev); void radeon_driver_irq_uninstall_kms(struct drm_device *dev); diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c index 535969d74f64..5bccdeae0773 100644 --- a/drivers/gpu/drm/radeon/radeon_kms.c +++ b/drivers/gpu/drm/radeon/radeon_kms.c @@ -874,7 +874,7 @@ void radeon_disable_vblank_kms(struct drm_device *dev, int crtc) bool radeon_get_vblank_timestamp_kms(struct drm_device *dev, int crtc, int *max_error, struct timeval *vblank_time, - unsigned flags) + bool in_vblank_irq) { struct drm_crtc *drmcrtc; struct radeon_device *rdev = dev->dev_private; @@ -891,7 +891,7 @@ bool radeon_get_vblank_timestamp_kms(struct drm_device *dev, int crtc,

/* Helper routine in DRM core does all the work: */ return drm_calc_vbltimestamp_from_scanoutpos(dev, crtc, max_error, - vblank_time, flags, + vblank_time, in_vblank_irq, &drmcrtc->hwmode); }

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index 60fefe9d7b8c..38f5cc740cd1 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -272,14 +272,14 @@ int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id,

bool vc4_crtc_get_vblank_timestamp(struct drm_device *dev, unsigned int crtc_id, int *max_error, struct timeval *vblank_time, - unsigned flags) + bool in_vblank_irq) { struct drm_crtc *crtc = drm_crtc_from_index(dev, crtc_id); struct drm_crtc_state *state = crtc->state;

/* Helper routine in DRM core does all the work: */ return drm_calc_vbltimestamp_from_scanoutpos(dev, crtc_id, max_error, - vblank_time, flags, + vblank_time, in_vblank_irq, &state->adjusted_mode); }

diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index 8a5d0e12ee02..815cdeb54971 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -452,7 +452,7 @@ int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id, const struct drm_display_mode *mode); bool vc4_crtc_get_vblank_timestamp(struct drm_device *dev, unsigned int crtc_id, int *max_error, struct timeval *vblank_time, - unsigned flags); + bool in_vblank_irq);

/* vc4_debugfs.c */ int vc4_debugfs_init(struct drm_minor *minor); diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index da78e248d9d8..9fe6301edd6a 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -308,11 +308,10 @@ struct drm_driver { * Returns true upper bound on error for timestamp. * vblank_time: * Target location for returned vblank timestamp. - * flags: - * 0 = Defaults, no special treatment needed. - * DRM_CALLED_FROM_VBLIRQ = Function is called from vblank - * irq handler. Some drivers need to apply some workarounds - * for gpu-specific vblank irq quirks if flag is set. + * 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 flag is set. * * Returns: * @@ -322,7 +321,7 @@ struct drm_driver { bool (*get_vblank_timestamp) (struct drm_device *dev, unsigned int pipe, int *max_error, struct timeval *vblank_time, - unsigned flags); + bool in_vblank_irq);

/* these have to be filled in */

diff --git a/include/drm/drm_irq.h b/include/drm/drm_irq.h index f0d5ccf9b282..445406efb8dc 100644 --- a/include/drm/drm_irq.h +++ b/include/drm/drm_irq.h @@ -156,7 +156,7 @@ u32 drm_accurate_vblank_count(struct drm_crtc *crtc); bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, unsigned int pipe, int *max_error, struct timeval *vblank_time, - unsigned flags, + bool in_vblank_irq, const struct drm_display_mode *mode); void drm_calc_timestamping_constants(struct drm_crtc *crtc, const struct drm_display_mode *mode);

This is going to be a bit too much, but good to have at least a small note about where this should all head towards.

Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- include/drm/drm_drv.h | 10 ++++++++++ 1 file changed, 10 insertions(+)

diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index 9fe6301edd6a..0a367cf5d8d5 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -274,6 +274,11 @@ struct drm_driver { * constant but unknown small number of scanlines wrt. real scanout * position. * + * 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. */ int (*get_scanout_position) (struct drm_device *dev, unsigned int pipe, unsigned int flags, int *vpos, int *hpos, @@ -317,6 +322,11 @@ struct drm_driver { * * True on success, false on failure, which means the core should * fallback to a simple timestamp taken in drm_crtc_handle_vblank(). + * + * FIXME: + * + * We should move this hook to &struct drm_crtc_funcs like all the other + * vblank hooks. */ bool (*get_vblank_timestamp) (struct drm_device *dev, unsigned int pipe, int *max_error,

If we restrict this helper to only kms drivers (which is the case) we can look up the correct mode easily ourselves. But it's a bit tricky:

- All legacy drivers look at crtc->hwmode. But that is update already at the beginning of the modeset helper, which means when we disable a pipe. Hence the final timestamps might be a bit off. But since this is an existing bug I'm not going to change it, but just try to be bug-for-bug compatible with the current code. This only applies to radeon&amdgpu.

- i915 tries to get it perfect by updating crtc->hwmode when the pipe is off (i.e. vblank->enabled = false).

- All other atomic drivers look at crtc->state->adjusted_mode. Those that look at state->requested_mode simply don't adjust their mode, so it's the same. That has two problems: Accessing crtc->state from interrupt handling code is unsafe, and it's updated before we shut down the pipe. For nonblocking modesets it's even worse.

For atomic drivers try to implement what i915 does. To do that we add a new hwmode field to the vblank structure, and update it from drm_calc_timestamping_constants(). For atomic drivers that's called from the right spot by the helper library already, so all fine. But for safety let's enforce that.

For legacy driver this function is only called at the end (oh the fun), which is broken, so again let's not bother and just stay bug-for-bug compatible.

The benefit is that we can use drm_calc_vbltimestamp_from_scanoutpos directly to implement ->get_vblank_timestamp in every driver, deleting a lot of code.

v2: Completely new approach, trying to mimick the i915 solution.

v3: Fixup kerneldoc.

v4: Drop the WARN_ON to check that the vblank is off, atomic helpers currently unconditionally call this. Recomputing the same stuff should be harmless.

Cc: Mario Kleiner mario.kleiner@tuebingen.mpg.de Cc: Eric Anholt eric@anholt.net Cc: Rob Clark robdclark@gmail.com Cc: linux-arm-msm@vger.kernel.org Cc: freedreno@lists.freedesktop.org Cc: Alex Deucher alexander.deucher@amd.com Cc: Christian König christian.koenig@amd.com Cc: Ben Skeggs bskeggs@redhat.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 4 --- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 41 ------------------------------- drivers/gpu/drm/drm_irq.c | 27 +++++++++++++++++--- drivers/gpu/drm/i915/i915_irq.c | 33 +------------------------ drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 26 +------------------- drivers/gpu/drm/nouveau/nouveau_display.c | 22 ----------------- drivers/gpu/drm/nouveau/nouveau_display.h | 2 -- drivers/gpu/drm/nouveau/nouveau_drm.c | 2 +- drivers/gpu/drm/radeon/radeon_drv.c | 6 +---- drivers/gpu/drm/radeon/radeon_kms.c | 37 ---------------------------- drivers/gpu/drm/vc4/vc4_crtc.c | 13 ---------- drivers/gpu/drm/vc4/vc4_drv.c | 2 +- drivers/gpu/drm/vc4/vc4_drv.h | 3 --- include/drm/drm_irq.h | 15 +++++++++-- 15 files changed, 42 insertions(+), 193 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 310b20ef794a..11ebe861ec78 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -1913,10 +1913,6 @@ int amdgpu_device_resume(struct drm_device *dev, bool resume, bool fbcon); u32 amdgpu_get_vblank_counter_kms(struct drm_device *dev, unsigned int pipe); int amdgpu_enable_vblank_kms(struct drm_device *dev, unsigned int pipe); void amdgpu_disable_vblank_kms(struct drm_device *dev, unsigned int pipe); -bool amdgpu_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe, - int *max_error, - struct timeval *vblank_time, - bool in_vblank_irq); long amdgpu_kms_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 400917fd7486..40dd5d19f835 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -725,7 +725,7 @@ static struct drm_driver kms_driver = { .get_vblank_counter = amdgpu_get_vblank_counter_kms, .enable_vblank = amdgpu_enable_vblank_kms, .disable_vblank = amdgpu_disable_vblank_kms, - .get_vblank_timestamp = amdgpu_get_vblank_timestamp_kms, + .get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos, .get_scanout_position = amdgpu_get_crtc_scanoutpos, #if defined(CONFIG_DEBUG_FS) .debugfs_init = amdgpu_debugfs_init, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c index 109a77ffc692..991ae253171b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c @@ -940,47 +940,6 @@ void amdgpu_disable_vblank_kms(struct drm_device *dev, unsigned int pipe) amdgpu_irq_put(adev, &adev->crtc_irq, idx); }

-/** - * amdgpu_get_vblank_timestamp_kms - get vblank timestamp - * - * @dev: drm dev pointer - * @crtc: crtc to get the timestamp for - * @max_error: max error - * @vblank_time: time value - * @in_vblank_irq: called from drm_handle_vblank() - * - * Gets the timestamp on the requested crtc based on the - * scanout position. (all asics). - * Returns true on success, false on failure. - */ -bool amdgpu_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe, - int *max_error, - struct timeval *vblank_time, - bool in_vblank_irq) -{ - struct drm_crtc *crtc; - struct amdgpu_device *adev = dev->dev_private; - - if (pipe >= dev->num_crtcs) { - DRM_ERROR("Invalid crtc %u\n", pipe); - return false; - } - - /* Get associated drm_crtc: */ - crtc = &adev->mode_info.crtcs[pipe]->base; - if (!crtc) { - /* This can occur on driver load if some component fails to - * initialize completely and driver is unloaded */ - DRM_ERROR("Uninitialized crtc %d\n", pipe); - return false; - } - - /* Helper routine in DRM core does all the work: */ - return drm_calc_vbltimestamp_from_scanoutpos(dev, pipe, max_error, - vblank_time, in_vblank_irq, - &crtc->hwmode); -} - const struct drm_ioctl_desc amdgpu_ioctls_kms[] = { DRM_IOCTL_DEF_DRV(AMDGPU_GEM_CREATE, amdgpu_gem_create_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), DRM_IOCTL_DEF_DRV(AMDGPU_CTX, amdgpu_ctx_ioctl, DRM_AUTH|DRM_RENDER_ALLOW), diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 82b6e0521fe1..992eb3719c3e 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -684,6 +684,7 @@ void drm_calc_timestamping_constants(struct drm_crtc *crtc,

vblank->linedur_ns = linedur_ns; vblank->framedur_ns = framedur_ns; + vblank->hwmode = *mode;

DRM_DEBUG("crtc %u: hwmode: htotal %d, vtotal %d, vdisplay %d\n", crtc->base.id, mode->crtc_htotal, @@ -704,7 +705,6 @@ EXPORT_SYMBOL(drm_calc_timestamping_constants); * True when called from drm_crtc_handle_vblank(). Some drivers * need to apply some workarounds for gpu-specific vblank irq quirks * if flag is set. - * @mode: mode which defines the scanout timings * * Implements calculation of exact vblank timestamps from given drm_display_mode * timings and current video scanout position of a CRTC. This can be called from @@ -724,6 +724,13 @@ EXPORT_SYMBOL(drm_calc_timestamping_constants); * returns as no operation if a doublescan or interlaced video mode is * active. Higher level code is expected to handle this. * + * This function can be used to implement the &drm_driver.get_vblank_timestamp + * directly, if the driver implements the &drm_driver.get_scanout_position hook. + * + * Note that atomic drivers must call drm_calc_timestamping_constants() before + * enabling a CRTC. The atomic helpers already take care of that in + * drm_atomic_helper_update_legacy_modeset_state(). + * * Returns: * * Returns true on success, and false on failure, i.e. when no accurate @@ -733,17 +740,24 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, unsigned int pipe, int *max_error, struct timeval *vblank_time, - bool in_vblank_irq, - const struct drm_display_mode *mode) + bool in_vblank_irq) { struct timeval tv_etime; ktime_t stime, etime; unsigned int vbl_status; + struct drm_crtc *crtc; + const struct drm_display_mode *mode; + struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; int vpos, hpos, i; int delta_ns, duration_ns; unsigned flags = in_vblank_irq ? DRM_CALLED_FROM_VBLIRQ : 0;

- if (pipe >= dev->num_crtcs) { + if (!drm_core_check_feature(dev, DRIVER_MODESET)) + return false; + + crtc = drm_crtc_from_index(dev, pipe); + + if (pipe >= dev->num_crtcs || !crtc) { DRM_ERROR("Invalid crtc %u\n", pipe); return false; } @@ -754,6 +768,11 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, return false; }

+ if (drm_drv_uses_atomic_modeset(dev)) + mode = &vblank->hwmode; + else + mode = &crtc->hwmode; + /* If mode timing undefined, just return as no-op: * Happens during initial modesetting of a crtc. */ diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 6e1063d3629e..fdb1162427a9 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -964,37 +964,6 @@ int intel_get_crtc_scanline(struct intel_crtc *crtc) return position; }

-static bool i915_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe, - int *max_error, - struct timeval *vblank_time, - bool in_vblank_irq) -{ - struct drm_i915_private *dev_priv = to_i915(dev); - struct intel_crtc *crtc; - - if (pipe >= INTEL_INFO(dev_priv)->num_pipes) { - DRM_ERROR("Invalid crtc %u\n", pipe); - return false; - } - - /* Get drm_crtc to timestamp: */ - crtc = intel_get_crtc_for_pipe(dev_priv, pipe); - if (crtc == NULL) { - DRM_ERROR("Invalid crtc %u\n", pipe); - return false; - } - - if (!crtc->base.hwmode.crtc_clock) { - DRM_DEBUG_KMS("crtc %u is disabled\n", pipe); - return false; - } - - /* Helper routine in DRM core does all the work: */ - return drm_calc_vbltimestamp_from_scanoutpos(dev, pipe, max_error, - vblank_time, in_vblank_irq, - &crtc->base.hwmode); -} - static void ironlake_rps_change_irq_handler(struct drm_i915_private *dev_priv) { u32 busy_up, busy_down, max_avg, min_avg; @@ -4329,7 +4298,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv)

dev_priv->hotplug.hpd_storm_threshold = HPD_STORM_DEFAULT_THRESHOLD;

- dev->driver->get_vblank_timestamp = i915_get_vblank_timestamp; + dev->driver->get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos; dev->driver->get_scanout_position = i915_get_crtc_scanoutpos;

if (IS_CHERRYVIEW(dev_priv)) { diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c index 16184ccbdd3b..6ba216b8bba9 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c @@ -595,30 +595,6 @@ static int mdp5_get_scanoutpos(struct drm_device *dev, unsigned int pipe, return ret; }

-static bool mdp5_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe, - int *max_error, - struct timeval *vblank_time, - bool in_vblank_irq) -{ - struct msm_drm_private *priv = dev->dev_private; - struct drm_crtc *crtc; - - if (pipe < 0 || pipe >= priv->num_crtcs) { - DRM_ERROR("Invalid crtc %d\n", pipe); - return false; - } - - crtc = priv->crtcs[pipe]; - if (!crtc) { - DRM_ERROR("Invalid crtc %d\n", pipe); - return false; - } - - return drm_calc_vbltimestamp_from_scanoutpos(dev, pipe, max_error, - vblank_time, in_vblank_irq, - &crtc->mode); -} - static u32 mdp5_get_vblank_counter(struct drm_device *dev, unsigned int pipe) { struct msm_drm_private *priv = dev->dev_private; @@ -728,7 +704,7 @@ struct msm_kms *mdp5_kms_init(struct drm_device *dev) dev->mode_config.max_width = 0xffff; dev->mode_config.max_height = 0xffff;

- dev->driver->get_vblank_timestamp = mdp5_get_vblank_timestamp; + dev->driver->get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos; dev->driver->get_scanout_position = mdp5_get_scanoutpos; dev->driver->get_vblank_counter = mdp5_get_vblank_counter; dev->max_vblank_count = 0xffffffff; diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c index c7caa0674c54..33d0a6175d44 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.c +++ b/drivers/gpu/drm/nouveau/nouveau_display.c @@ -156,28 +156,6 @@ nouveau_display_scanoutpos(struct drm_device *dev, unsigned int pipe, return 0; }

-bool -nouveau_display_vblstamp(struct drm_device *dev, unsigned int pipe, - int *max_error, struct timeval *time, bool in_vblank_irq) -{ - struct drm_crtc *crtc; - - list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { - if (nouveau_crtc(crtc)->index == pipe) { - struct drm_display_mode *mode; - if (drm_drv_uses_atomic_modeset(dev)) - mode = &crtc->state->adjusted_mode; - else - mode = &crtc->hwmode; - return drm_calc_vbltimestamp_from_scanoutpos(dev, - pipe, max_error, time, in_vblank_irq, - mode); - } - } - - return false; -} - static void nouveau_display_vblank_fini(struct drm_device *dev) { diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h b/drivers/gpu/drm/nouveau/nouveau_display.h index 8ec86259c5ac..a8285c2747a9 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.h +++ b/drivers/gpu/drm/nouveau/nouveau_display.h @@ -71,8 +71,6 @@ void nouveau_display_vblank_disable(struct drm_device *, unsigned int); int nouveau_display_scanoutpos(struct drm_device *, unsigned int, unsigned int, int *, int *, ktime_t *, ktime_t *, const struct drm_display_mode *); -bool nouveau_display_vblstamp(struct drm_device *, unsigned int, int *, - struct timeval *, bool);

int nouveau_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb, struct drm_pending_vblank_event *event, diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c index ec719df619a6..1f751a3f570c 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drm.c +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c @@ -978,7 +978,7 @@ driver_stub = { .enable_vblank = nouveau_display_vblank_enable, .disable_vblank = nouveau_display_vblank_disable, .get_scanout_position = nouveau_display_scanoutpos, - .get_vblank_timestamp = nouveau_display_vblstamp, + .get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos,

.ioctls = nouveau_ioctls, .num_ioctls = ARRAY_SIZE(nouveau_ioctls), diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c index 88fc791ec8fb..50d97f12e7bf 100644 --- a/drivers/gpu/drm/radeon/radeon_drv.c +++ b/drivers/gpu/drm/radeon/radeon_drv.c @@ -115,10 +115,6 @@ int radeon_resume_kms(struct drm_device *dev, bool resume, bool fbcon); u32 radeon_get_vblank_counter_kms(struct drm_device *dev, unsigned int pipe); int radeon_enable_vblank_kms(struct drm_device *dev, unsigned int pipe); void radeon_disable_vblank_kms(struct drm_device *dev, unsigned int pipe); -bool radeon_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe, - int *max_error, - struct timeval *vblank_time, - bool in_vblank_irq); void radeon_driver_irq_preinstall_kms(struct drm_device *dev); int radeon_driver_irq_postinstall_kms(struct drm_device *dev); void radeon_driver_irq_uninstall_kms(struct drm_device *dev); @@ -544,7 +540,7 @@ static struct drm_driver kms_driver = { .get_vblank_counter = radeon_get_vblank_counter_kms, .enable_vblank = radeon_enable_vblank_kms, .disable_vblank = radeon_disable_vblank_kms, - .get_vblank_timestamp = radeon_get_vblank_timestamp_kms, + .get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos, .get_scanout_position = radeon_get_crtc_scanoutpos, .irq_preinstall = radeon_driver_irq_preinstall_kms, .irq_postinstall = radeon_driver_irq_postinstall_kms, diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c index 5bccdeae0773..6a68d440bc44 100644 --- a/drivers/gpu/drm/radeon/radeon_kms.c +++ b/drivers/gpu/drm/radeon/radeon_kms.c @@ -858,43 +858,6 @@ void radeon_disable_vblank_kms(struct drm_device *dev, int crtc) spin_unlock_irqrestore(&rdev->irq.lock, irqflags); }

-/** - * radeon_get_vblank_timestamp_kms - get vblank timestamp - * - * @dev: drm dev pointer - * @crtc: crtc to get the timestamp for - * @max_error: max error - * @vblank_time: time value - * @flags: flags passed to the driver - * - * Gets the timestamp on the requested crtc based on the - * scanout position. (all asics). - * Returns true on success, false on failure. - */ -bool radeon_get_vblank_timestamp_kms(struct drm_device *dev, int crtc, - int *max_error, - struct timeval *vblank_time, - bool in_vblank_irq) -{ - struct drm_crtc *drmcrtc; - struct radeon_device *rdev = dev->dev_private; - - if (crtc < 0 || crtc >= dev->num_crtcs) { - DRM_ERROR("Invalid crtc %d\n", crtc); - return false; - } - - /* Get associated drm_crtc: */ - drmcrtc = &rdev->mode_info.crtcs[crtc]->base; - if (!drmcrtc) - return false; - - /* Helper routine in DRM core does all the work: */ - return drm_calc_vbltimestamp_from_scanoutpos(dev, crtc, max_error, - vblank_time, in_vblank_irq, - &drmcrtc->hwmode); -} - const struct drm_ioctl_desc radeon_ioctls_kms[] = { DRM_IOCTL_DEF_DRV(RADEON_CP_INIT, drm_invalid_op, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), DRM_IOCTL_DEF_DRV(RADEON_CP_START, drm_invalid_op, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index 38f5cc740cd1..2acfecc5b811 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -270,19 +270,6 @@ int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id, return ret; }

-bool vc4_crtc_get_vblank_timestamp(struct drm_device *dev, unsigned int crtc_id, - int *max_error, struct timeval *vblank_time, - bool in_vblank_irq) -{ - struct drm_crtc *crtc = drm_crtc_from_index(dev, crtc_id); - struct drm_crtc_state *state = crtc->state; - - /* Helper routine in DRM core does all the work: */ - return drm_calc_vbltimestamp_from_scanoutpos(dev, crtc_id, max_error, - vblank_time, in_vblank_irq, - &state->adjusted_mode); -} - static void vc4_crtc_destroy(struct drm_crtc *crtc) { drm_crtc_cleanup(crtc); diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c index 61e674baf3a6..e864256e12e5 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.c +++ b/drivers/gpu/drm/vc4/vc4_drv.c @@ -154,7 +154,7 @@ static struct drm_driver vc4_drm_driver = { .irq_uninstall = vc4_irq_uninstall,

.get_scanout_position = vc4_crtc_get_scanoutpos, - .get_vblank_timestamp = vc4_crtc_get_vblank_timestamp, + .get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos,

#if defined(CONFIG_DEBUG_FS) .debugfs_init = vc4_debugfs_init, diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index 815cdeb54971..64c92e0eb8f7 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -450,9 +450,6 @@ int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id, unsigned int flags, int *vpos, int *hpos, ktime_t *stime, ktime_t *etime, const struct drm_display_mode *mode); -bool vc4_crtc_get_vblank_timestamp(struct drm_device *dev, unsigned int crtc_id, - int *max_error, struct timeval *vblank_time, - bool in_vblank_irq);

/* vc4_debugfs.c */ int vc4_debugfs_init(struct drm_minor *minor); diff --git a/include/drm/drm_irq.h b/include/drm/drm_irq.h index 445406efb8dc..b489cc856e7a 100644 --- a/include/drm/drm_irq.h +++ b/include/drm/drm_irq.h @@ -121,6 +121,18 @@ struct drm_vblank_crtc { * drm_calc_timestamping_constants(). */ int linedur_ns; + + /** + * @hwmode: + * + * Cache of the current hardware display mode. Only valide when @enabled + * is set. This is used by helpers like + * drm_calc_vbltimestamp_from_scanoutpos(). We can't just access the + * hardware mode by e.g. looking at &drm_crtc_state.adjusted_mode, + * because that one is really hard to get at from interrupt context. + */ + struct drm_display_mode hwmode; + /** * @enabled: Tracks the enabling state of the corresponding &drm_crtc to * avoid double-disabling and hence corrupting saved state. Needed by @@ -156,8 +168,7 @@ u32 drm_accurate_vblank_count(struct drm_crtc *crtc); bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, unsigned int pipe, int *max_error, struct timeval *vblank_time, - bool in_vblank_irq, - const struct drm_display_mode *mode); + bool in_vblank_irq); void drm_calc_timestamping_constants(struct drm_crtc *crtc, const struct drm_display_mode *mode);

- We can drop the different return value flags, the only caller only cares about whether the scanout position is valid or not. Also, it's entirely undefined what "accurate" means, if we'd really care we should probably wire the max_error through. But since we never even report this to userspace it's kinda moot.

- Drop all the fancy input flags, there's really only the "called from vblank irq" one. Well except for radeon/amdgpu, which added their own private flags.

Since amdgpu/radoen also use the scanoutposition function internally I just gave them a tiny wrapper, plus copies of all the old #defines they need. Everyone else gets simplified code.

Note how we could remove a lot of error conditions if we'd move this helper hook to drm_crtc_helper_funcs and would pass it a crtc directly.

v2: Make it compile on arm.

v3: Squash in fixup from 0day.

Cc: Mario Kleiner mario.kleiner@tuebingen.mpg.de Cc: Eric Anholt eric@anholt.net Cc: Rob Clark robdclark@gmail.com Cc: linux-arm-msm@vger.kernel.org Cc: freedreno@lists.freedesktop.org Cc: Alex Deucher alexander.deucher@amd.com Cc: Christian König christian.koenig@amd.com Cc: Ben Skeggs bskeggs@redhat.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 12 +++++++++++- drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 3 +++ drivers/gpu/drm/drm_irq.c | 16 ++++++++-------- drivers/gpu/drm/i915/i915_irq.c | 19 ++++++------------- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 19 +++++++------------ drivers/gpu/drm/nouveau/nouveau_display.c | 18 ++++++++---------- drivers/gpu/drm/nouveau/nouveau_display.h | 6 +++--- drivers/gpu/drm/radeon/radeon_drv.c | 12 +++++++++++- drivers/gpu/drm/radeon/radeon_mode.h | 3 +++ drivers/gpu/drm/vc4/vc4_crtc.c | 21 ++++++++++----------- drivers/gpu/drm/vc4/vc4_drv.h | 8 ++++---- include/drm/drmP.h | 8 -------- include/drm/drm_drv.h | 26 ++++++++++---------------- 13 files changed, 84 insertions(+), 87 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index 40dd5d19f835..98bde673ae3e 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -711,6 +711,16 @@ static const struct file_operations amdgpu_driver_kms_fops = { #endif };

+static bool +amdgpu_get_crtc_scanout_position(struct drm_device *dev, unsigned int pipe, + bool in_vblank_irq, int *vpos, int *hpos, + ktime_t *stime, ktime_t *etime, + const struct drm_display_mode *mode) +{ + return amdgpu_get_crtc_scanoutpos(dev, pipe, 0, vpos, hpos, + stime, etime, mode); +} + static struct drm_driver kms_driver = { .driver_features = DRIVER_USE_AGP | @@ -726,7 +736,7 @@ static struct drm_driver kms_driver = { .enable_vblank = amdgpu_enable_vblank_kms, .disable_vblank = amdgpu_disable_vblank_kms, .get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos, - .get_scanout_position = amdgpu_get_crtc_scanoutpos, + .get_scanout_position = amdgpu_get_crtc_scanout_position, #if defined(CONFIG_DEBUG_FS) .debugfs_init = amdgpu_debugfs_init, #endif diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h index db8f8dda209c..20d6522fd7b4 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h @@ -534,6 +534,9 @@ struct amdgpu_framebuffer { ((em) == ATOM_ENCODER_MODE_DP_MST))

/* Driver internal use only flags of amdgpu_get_crtc_scanoutpos() */ +#define DRM_SCANOUTPOS_VALID (1 << 0) +#define DRM_SCANOUTPOS_IN_VBLANK (1 << 1) +#define DRM_SCANOUTPOS_ACCURATE (1 << 2) #define USE_REAL_VBLANKSTART (1 << 30) #define GET_DISTANCE_TO_VBLANKSTART (1 << 31)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 992eb3719c3e..6e04d05455f9 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -744,13 +744,12 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, { struct timeval tv_etime; ktime_t stime, etime; - unsigned int vbl_status; + bool vbl_status; struct drm_crtc *crtc; const struct drm_display_mode *mode; struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; int vpos, hpos, i; int delta_ns, duration_ns; - unsigned flags = in_vblank_irq ? DRM_CALLED_FROM_VBLIRQ : 0;

if (!drm_core_check_feature(dev, DRIVER_MODESET)) return false; @@ -793,15 +792,16 @@ 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, flags, + 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 & DRM_SCANOUTPOS_VALID)) { - DRM_DEBUG("crtc %u : scanoutpos query failed [0x%x].\n", - pipe, vbl_status); + if (!vbl_status) { + DRM_DEBUG("crtc %u : scanoutpos query failed.\n", + pipe); return false; }

@@ -840,8 +840,8 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, etime = ktime_sub_ns(etime, delta_ns); *vblank_time = ktime_to_timeval(etime);

- DRM_DEBUG_VBL("crtc %u : v 0x%x p(%d,%d)@ %ld.%ld -> %ld.%ld [e %d us, %d rep]\n", - pipe, vbl_status, hpos, vpos, + DRM_DEBUG_VBL("crtc %u : v p(%d,%d)@ %ld.%ld -> %ld.%ld [e %d us, %d rep]\n", + pipe, hpos, vpos, (long)tv_etime.tv_sec, (long)tv_etime.tv_usec, (long)vblank_time->tv_sec, (long)vblank_time->tv_usec, duration_ns/1000, i); diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index fdb1162427a9..6477bbd85772 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -827,10 +827,10 @@ static int __intel_get_crtc_scanline(struct intel_crtc *crtc) return (position + crtc->scanline_offset) % vtotal; }

-static int i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe, - unsigned int flags, int *vpos, int *hpos, - ktime_t *stime, ktime_t *etime, - const struct drm_display_mode *mode) +static bool i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe, + bool in_vblank_irq, int *vpos, int *hpos, + ktime_t *stime, ktime_t *etime, + const struct drm_display_mode *mode) { struct drm_i915_private *dev_priv = to_i915(dev); struct intel_crtc *intel_crtc = intel_get_crtc_for_pipe(dev_priv, @@ -838,13 +838,12 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe, int position; int vbl_start, vbl_end, hsync_start, htotal, vtotal; bool in_vbl = true; - int ret = 0; unsigned long irqflags;

if (WARN_ON(!mode->crtc_clock)) { DRM_DEBUG_DRIVER("trying to get scanoutpos for disabled " "pipe %c\n", pipe_name(pipe)); - return 0; + return false; }

htotal = mode->crtc_htotal; @@ -859,8 +858,6 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe, vtotal /= 2; }

- ret |= DRM_SCANOUTPOS_VALID | DRM_SCANOUTPOS_ACCURATE; - /* * Lock uncore.lock, as we will do multiple timing critical raw * register reads, potentially with preemption disabled, so the @@ -944,11 +941,7 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe, *hpos = position - (*vpos * htotal); }

- /* In vblank? */ - if (in_vbl) - ret |= DRM_SCANOUTPOS_IN_VBLANK; - - return ret; + return true; }

int intel_get_crtc_scanline(struct intel_crtc *crtc) diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c index 6ba216b8bba9..959b826123a2 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c @@ -530,31 +530,28 @@ static struct drm_encoder *get_encoder_from_crtc(struct drm_crtc *crtc) return NULL; }

-static int mdp5_get_scanoutpos(struct drm_device *dev, unsigned int pipe, - unsigned int flags, int *vpos, int *hpos, - ktime_t *stime, ktime_t *etime, - const struct drm_display_mode *mode) +static bool mdp5_get_scanoutpos(struct drm_device *dev, unsigned int pipe, + bool in_vblank_irq, int *vpos, int *hpos, + ktime_t *stime, ktime_t *etime, + const struct drm_display_mode *mode) { struct msm_drm_private *priv = dev->dev_private; struct drm_crtc *crtc; struct drm_encoder *encoder; int line, vsw, vbp, vactive_start, vactive_end, vfp_end; - int ret = 0;

crtc = priv->crtcs[pipe]; if (!crtc) { DRM_ERROR("Invalid crtc %d\n", pipe); - return 0; + return false; }

encoder = get_encoder_from_crtc(crtc); if (!encoder) { DRM_ERROR("no encoder found for crtc %d\n", pipe); - return 0; + return false; }

- ret |= DRM_SCANOUTPOS_VALID | DRM_SCANOUTPOS_ACCURATE; - vsw = mode->crtc_vsync_end - mode->crtc_vsync_start; vbp = mode->crtc_vtotal - mode->crtc_vsync_end;

@@ -578,10 +575,8 @@ static int mdp5_get_scanoutpos(struct drm_device *dev, unsigned int pipe,

if (line < vactive_start) { line -= vactive_start; - ret |= DRM_SCANOUTPOS_IN_VBLANK; } else if (line > vactive_end) { line = line - vfp_end - vactive_start; - ret |= DRM_SCANOUTPOS_IN_VBLANK; } else { line -= vactive_start; } @@ -592,7 +587,7 @@ static int mdp5_get_scanoutpos(struct drm_device *dev, unsigned int pipe, if (etime) *etime = ktime_get();

- return ret; + return true; }

static u32 mdp5_get_vblank_counter(struct drm_device *dev, unsigned int pipe) diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c index 33d0a6175d44..16110be3aba8 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.c +++ b/drivers/gpu/drm/nouveau/nouveau_display.c @@ -98,7 +98,7 @@ calc(int blanks, int blanke, int total, int line) return line; }

-static int +static bool nouveau_display_scanoutpos_head(struct drm_crtc *crtc, int *vpos, int *hpos, ktime_t *stime, ktime_t *etime) { @@ -111,16 +111,16 @@ nouveau_display_scanoutpos_head(struct drm_crtc *crtc, int *vpos, int *hpos, }; struct nouveau_display *disp = nouveau_display(crtc->dev); struct drm_vblank_crtc *vblank = &crtc->dev->vblank[drm_crtc_index(crtc)]; - int ret, retry = 1; + int retry = 1; + bool ret = false;

do { ret = nvif_mthd(&disp->disp, 0, &args, sizeof(args)); if (ret != 0) - return 0; + return false;

if (args.scan.vline) { - ret |= DRM_SCANOUTPOS_ACCURATE; - ret |= DRM_SCANOUTPOS_VALID; + ret = true; break; }

@@ -133,14 +133,12 @@ nouveau_display_scanoutpos_head(struct drm_crtc *crtc, int *vpos, int *hpos, if (stime) *stime = ns_to_ktime(args.scan.time[0]); if (etime) *etime = ns_to_ktime(args.scan.time[1]);

- if (*vpos < 0) - ret |= DRM_SCANOUTPOS_IN_VBLANK; return ret; }

-int +bool nouveau_display_scanoutpos(struct drm_device *dev, unsigned int pipe, - unsigned int flags, int *vpos, int *hpos, + bool in_vblank_irq, int *vpos, int *hpos, ktime_t *stime, ktime_t *etime, const struct drm_display_mode *mode) { @@ -153,7 +151,7 @@ nouveau_display_scanoutpos(struct drm_device *dev, unsigned int pipe, } }

- return 0; + return false; }

static void diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h b/drivers/gpu/drm/nouveau/nouveau_display.h index a8285c2747a9..201aec2ea5b8 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.h +++ b/drivers/gpu/drm/nouveau/nouveau_display.h @@ -68,9 +68,9 @@ int nouveau_display_suspend(struct drm_device *dev, bool runtime); void nouveau_display_resume(struct drm_device *dev, bool runtime); int nouveau_display_vblank_enable(struct drm_device *, unsigned int); void nouveau_display_vblank_disable(struct drm_device *, unsigned int); -int nouveau_display_scanoutpos(struct drm_device *, unsigned int, - unsigned int, int *, int *, ktime_t *, - ktime_t *, const struct drm_display_mode *); +bool nouveau_display_scanoutpos(struct drm_device *, unsigned int, + bool, int *, int *, ktime_t *, + ktime_t *, const struct drm_display_mode *);

int nouveau_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb, struct drm_pending_vblank_event *event, diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c index 50d97f12e7bf..ef8a75940980 100644 --- a/drivers/gpu/drm/radeon/radeon_drv.c +++ b/drivers/gpu/drm/radeon/radeon_drv.c @@ -526,6 +526,16 @@ static const struct file_operations radeon_driver_kms_fops = { #endif };

+static bool +radeon_get_crtc_scanout_position(struct drm_device *dev, unsigned int pipe, + bool in_vblank_irq, int *vpos, int *hpos, + ktime_t *stime, ktime_t *etime, + const struct drm_display_mode *mode) +{ + return radeon_get_crtc_scanoutpos(dev, pipe, 0, vpos, hpos, + stime, etime, mode); +} + static struct drm_driver kms_driver = { .driver_features = DRIVER_USE_AGP | @@ -541,7 +551,7 @@ static struct drm_driver kms_driver = { .enable_vblank = radeon_enable_vblank_kms, .disable_vblank = radeon_disable_vblank_kms, .get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos, - .get_scanout_position = radeon_get_crtc_scanoutpos, + .get_scanout_position = radeon_get_crtc_scanout_position, .irq_preinstall = radeon_driver_irq_preinstall_kms, .irq_postinstall = radeon_driver_irq_postinstall_kms, .irq_uninstall = radeon_driver_irq_uninstall_kms, diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h index ad282648fc8b..00f5ec5c12c7 100644 --- a/drivers/gpu/drm/radeon/radeon_mode.h +++ b/drivers/gpu/drm/radeon/radeon_mode.h @@ -691,6 +691,9 @@ struct atom_voltage_table };

/* Driver internal use only flags of radeon_get_crtc_scanoutpos() */ +#define DRM_SCANOUTPOS_VALID (1 << 0) +#define DRM_SCANOUTPOS_IN_VBLANK (1 << 1) +#define DRM_SCANOUTPOS_ACCURATE (1 << 2) #define USE_REAL_VBLANKSTART (1 << 30) #define GET_DISTANCE_TO_VBLANKSTART (1 << 31)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index 2acfecc5b811..04c390a487ba 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -151,10 +151,10 @@ int vc4_crtc_debugfs_regs(struct seq_file *m, void *unused) } #endif

-int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id, - unsigned int flags, int *vpos, int *hpos, - ktime_t *stime, ktime_t *etime, - const struct drm_display_mode *mode) +bool vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id, + bool in_vblank_irq, int *vpos, int *hpos, + ktime_t *stime, ktime_t *etime, + const struct drm_display_mode *mode) { struct vc4_dev *vc4 = to_vc4_dev(dev); struct drm_crtc *crtc = drm_crtc_from_index(dev, crtc_id); @@ -162,7 +162,7 @@ int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id, u32 val; int fifo_lines; int vblank_lines; - int ret = 0; + bool ret = false;

/* preempt_disable_rt() should go right here in PREEMPT_RT patchset. */

@@ -198,7 +198,7 @@ int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id, fifo_lines = vc4_crtc->cob_size / mode->crtc_hdisplay;

if (fifo_lines > 0) - ret |= DRM_SCANOUTPOS_VALID; + ret = true;

/* HVS more than fifo_lines into frame for compositing? */ if (*vpos > fifo_lines) { @@ -216,7 +216,6 @@ int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id, */ *vpos -= fifo_lines + 1;

- ret |= DRM_SCANOUTPOS_ACCURATE; return ret; }

@@ -229,10 +228,9 @@ int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id, * We can't get meaningful readings wrt. scanline position of the PV * and need to make things up in a approximative but consistent way. */ - ret |= DRM_SCANOUTPOS_IN_VBLANK; vblank_lines = mode->vtotal - mode->vdisplay;

- if (flags & DRM_CALLED_FROM_VBLIRQ) { + if (in_vblank_irq) { /* * Assume the irq handler got called close to first * line of vblank, so PV has about a full vblank @@ -254,9 +252,10 @@ int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id, * we are at the very beginning of vblank, as the hvs just * started refilling, and the stime and etime timestamps * truly correspond to start of vblank. + * + * Unfortunately there's no way to report this to upper levels + * and make it more useful. */ - if ((val & SCALER_DISPSTATX_FULL) != SCALER_DISPSTATX_FULL) - ret |= DRM_SCANOUTPOS_ACCURATE; } else { /* * No clue where we are inside vblank. Return a vpos of zero, diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index 64c92e0eb8f7..c34a0915e49d 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -446,10 +446,10 @@ int vc4_bo_stats_debugfs(struct seq_file *m, void *arg); extern struct platform_driver vc4_crtc_driver; bool vc4_event_pending(struct drm_crtc *crtc); int vc4_crtc_debugfs_regs(struct seq_file *m, void *arg); -int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id, - unsigned int flags, int *vpos, int *hpos, - ktime_t *stime, ktime_t *etime, - const struct drm_display_mode *mode); +bool vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id, + bool in_vblank_irq, int *vpos, int *hpos, + ktime_t *stime, ktime_t *etime, + const struct drm_display_mode *mode);

/* vc4_debugfs.c */ int vc4_debugfs_init(struct drm_minor *minor); diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 1b3f3cd7b230..6c7ea497e3a6 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -321,14 +321,6 @@ struct pci_controller; #define DRM_IF_VERSION(maj, min) (maj << 16 | min)

-/* Flags and return codes for get_vblank_timestamp() driver function. */ -#define DRM_CALLED_FROM_VBLIRQ 1 - -/* get_scanout_position() return flags */ -#define DRM_SCANOUTPOS_VALID (1 << 0) -#define DRM_SCANOUTPOS_IN_VBLANK (1 << 1) -#define DRM_SCANOUTPOS_ACCURATE (1 << 2) - /** * DRM device structure. This structure represent a complete card that * may contain multiple heads. diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index 0a367cf5d8d5..e64e33b9dd26 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -241,8 +241,10 @@ struct drm_driver { * DRM device. * pipe: * Id of the crtc to query. - * flags: - * Flags from the caller (DRM_CALLED_FROM_VBLIRQ or 0). + * 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 flag is set. * vpos: * Target location for current vertical scanout position. * hpos: @@ -263,16 +265,8 @@ struct drm_driver { * * Returns: * - * Flags, or'ed together as follows: - * - * DRM_SCANOUTPOS_VALID: - * Query successful. - * DRM_SCANOUTPOS_INVBL: - * Inside vblank. - * DRM_SCANOUTPOS_ACCURATE: Returned position is accurate. A lack of - * this flag means that returned position may be offset by a - * constant but unknown small number of scanlines wrt. real scanout - * position. + * True on success, false if a reliable scanout position counter could + * not be read out. * * FIXME: * @@ -280,10 +274,10 @@ struct drm_driver { * move it to &struct drm_crtc_helper_funcs, like all the other * helper-internal hooks. */ - int (*get_scanout_position) (struct drm_device *dev, unsigned int pipe, - unsigned int flags, int *vpos, int *hpos, - ktime_t *stime, ktime_t *etime, - const struct drm_display_mode *mode); + bool (*get_scanout_position) (struct drm_device *dev, unsigned int pipe, + bool in_vblank_irq, int *vpos, int *hpos, + ktime_t *stime, ktime_t *etime, + const struct drm_display_mode *mode);

/** * @get_vblank_timestamp:

In the previous patch we've implemented hwmode tracking a la i915 for the vblank timestamp calculations. But that was just the basic semantics, i915 has some nice sanity checks to make sure we keep getting this right. Move them over too.

Cc: Ville Syrjälä ville.syrjala@linux.intel.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- drivers/gpu/drm/drm_irq.c | 8 +++++++- drivers/gpu/drm/i915/i915_irq.c | 10 ++++++---- drivers/gpu/drm/i915/intel_display.c | 11 ++--------- 3 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 6e04d05455f9..41e9818666b8 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -775,8 +775,10 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, /* If mode timing undefined, just return as no-op: * Happens during initial modesetting of a crtc. */ - if (mode->crtc_clock == 0) { + if (WARN_ON(mode->crtc_clock == 0)) { DRM_DEBUG("crtc %u: Noop due to uninitialized mode.\n", pipe); + WARN_ON(drm_drv_uses_atomic_modeset(dev)); + return false; }

@@ -1336,6 +1338,10 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc) send_vblank_event(dev, e, seq, &now); } spin_unlock_irqrestore(&dev->event_lock, irqflags); + + /* Will be reset by the modeset helpers when re-enabling the crtc by + * calling drm_calc_timestamping_constants(). */ + vblank->hwmode.crtc_clock = 0; } EXPORT_SYMBOL(drm_crtc_vblank_off);

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 6477bbd85772..a6046e3e36f5 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -720,9 +720,7 @@ static u32 i915_get_vblank_counter(struct drm_device *dev, unsigned int pipe) struct drm_i915_private *dev_priv = to_i915(dev); i915_reg_t high_frame, low_frame; u32 high1, high2, low, pixel, vbl_start, hsync_start, htotal; - struct intel_crtc *intel_crtc = intel_get_crtc_for_pipe(dev_priv, - pipe); - const struct drm_display_mode *mode = &intel_crtc->base.hwmode; + const struct drm_display_mode *mode = &dev->vblank[pipe].hwmode; unsigned long irqflags;

htotal = mode->crtc_htotal; @@ -779,13 +777,17 @@ static int __intel_get_crtc_scanline(struct intel_crtc *crtc) { struct drm_device *dev = crtc->base.dev; struct drm_i915_private *dev_priv = to_i915(dev); - const struct drm_display_mode *mode = &crtc->base.hwmode; + const struct drm_display_mode *mode; + struct drm_vblank_crtc *vblank; enum pipe pipe = crtc->pipe; int position, vtotal;

if (!crtc->active) return -1;

+ vblank = &crtc->base.dev->vblank[drm_crtc_index(&crtc->base)]; + mode = &vblank->hwmode; + vtotal = mode->crtc_vtotal; if (mode->flags & DRM_MODE_FLAG_INTERLACE) vtotal /= 2; diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 81baa5a9780c..53e732d36fda 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11461,12 +11461,6 @@ intel_modeset_update_crtc_state(struct drm_atomic_state *state) for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { to_intel_crtc(crtc)->config = to_intel_crtc_state(new_crtc_state);

- /* Update hwmode for vblank functions */ - if (new_crtc_state->active) - crtc->hwmode = new_crtc_state->adjusted_mode; - else - crtc->hwmode.crtc_clock = 0; - /* * Update legacy state to satisfy fbc code. This can * be removed when fbc uses the atomic state. @@ -15471,8 +15465,6 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) to_intel_crtc_state(crtc->base.state); int pixclk = 0;

- crtc->base.hwmode = crtc_state->base.adjusted_mode; - memset(&crtc->base.mode, 0, sizeof(crtc->base.mode)); if (crtc_state->base.active) { intel_mode_from_pipe_config(&crtc->base.mode, crtc_state); @@ -15502,7 +15494,8 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled) pixclk = DIV_ROUND_UP(pixclk * 100, 95);

- drm_calc_timestamping_constants(&crtc->base, &crtc->base.hwmode); + drm_calc_timestamping_constants(&crtc->base, + &crtc_state->base.adjusted_mode); update_scanline_offset(crtc); }

Drive-by cleanup.

Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- drivers/gpu/drm/drm_probe_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 85005d57bde6..efb5e5e8ce62 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -44,7 +44,7 @@ * * This library provides some helper code for output probing. It provides an * implementation of the core &drm_connector_funcs.fill_modes interface with - * drm_helper_probe_single_connector_modes. + * drm_helper_probe_single_connector_modes(). * * It also provides support for polling connectors with a work item and for * generic hotplug interrupt handling where the driver doesn't or cannot keep