[1/2] drm/atomic: Allow drivers to write their own plane check for async flips
Commit Message
Some hardware are more flexible on what they can flip asynchronously, so
rework the plane check so drivers can implement their own check, lifting
up some of the restrictions.
Signed-off-by: André Almeida <andrealmeid@igalia.com>
---
drivers/gpu/drm/drm_atomic_uapi.c | 62 ++++++++++++++++++++++---------
include/drm/drm_atomic_uapi.h | 12 ++++++
include/drm/drm_plane.h | 5 +++
3 files changed, 62 insertions(+), 17 deletions(-)
Comments
Hi André,
kernel test robot noticed the following build warnings:
[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next drm-tip/drm-tip linus/master next-20240119]
[cannot apply to drm-intel/for-linux-next-fixes v6.7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Andr-Almeida/drm-atomic-Allow-drivers-to-write-their-own-plane-check-for-async-flips/20240116-125406
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link: https://lore.kernel.org/r/20240116045159.1015510-2-andrealmeid%40igalia.com
patch subject: [PATCH 1/2] drm/atomic: Allow drivers to write their own plane check for async flips
config: hexagon-randconfig-002-20240119 (https://download.01.org/0day-ci/archive/20240120/202401200526.Xggix2DE-lkp@intel.com/config)
compiler: clang version 18.0.0git (https://github.com/llvm/llvm-project d92ce344bf641e6bb025b41b3f1a77dd25e2b3e9)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240120/202401200526.Xggix2DE-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401200526.Xggix2DE-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/gpu/drm/drm_atomic_uapi.c:31:
In file included from include/drm/drm_atomic.h:31:
In file included from include/drm/drm_crtc.h:32:
In file included from include/drm/drm_modes.h:33:
In file included from include/drm/drm_connector.h:32:
In file included from include/drm/drm_util.h:35:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:337:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
547 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
560 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from drivers/gpu/drm/drm_atomic_uapi.c:31:
In file included from include/drm/drm_atomic.h:31:
In file included from include/drm/drm_crtc.h:32:
In file included from include/drm/drm_modes.h:33:
In file included from include/drm/drm_connector.h:32:
In file included from include/drm/drm_util.h:35:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:337:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
573 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from drivers/gpu/drm/drm_atomic_uapi.c:31:
In file included from include/drm/drm_atomic.h:31:
In file included from include/drm/drm_crtc.h:32:
In file included from include/drm/drm_modes.h:33:
In file included from include/drm/drm_connector.h:32:
In file included from include/drm/drm_util.h:35:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:337:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
584 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
594 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
604 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
>> drivers/gpu/drm/drm_atomic_uapi.c:1149:30: warning: variable 'old_val' is uninitialized when used here [-Wuninitialized]
1149 | obj, prop_value, old_val);
| ^~~~~~~
drivers/gpu/drm/drm_atomic_uapi.c:1087:13: note: initialize the variable 'old_val' to silence this warning
1087 | u64 old_val;
| ^
| = 0
7 warnings generated.
vim +/old_val +1149 drivers/gpu/drm/drm_atomic_uapi.c
1078
1079 int drm_atomic_set_property(struct drm_atomic_state *state,
1080 struct drm_file *file_priv,
1081 struct drm_mode_object *obj,
1082 struct drm_property *prop,
1083 u64 prop_value,
1084 bool async_flip)
1085 {
1086 struct drm_mode_object *ref;
1087 u64 old_val;
1088 int ret;
1089
1090 if (!drm_property_change_valid_get(prop, prop_value, &ref))
1091 return -EINVAL;
1092
1093 switch (obj->type) {
1094 case DRM_MODE_OBJECT_CONNECTOR: {
1095 struct drm_connector *connector = obj_to_connector(obj);
1096 struct drm_connector_state *connector_state;
1097
1098 connector_state = drm_atomic_get_connector_state(state, connector);
1099 if (IS_ERR(connector_state)) {
1100 ret = PTR_ERR(connector_state);
1101 break;
1102 }
1103
1104 if (async_flip) {
1105 ret = drm_atomic_connector_get_property(connector, connector_state,
1106 prop, &old_val);
1107 ret = drm_atomic_check_prop_changes(ret, old_val, prop_value, prop);
1108 break;
1109 }
1110
1111 ret = drm_atomic_connector_set_property(connector,
1112 connector_state, file_priv,
1113 prop, prop_value);
1114 break;
1115 }
1116 case DRM_MODE_OBJECT_CRTC: {
1117 struct drm_crtc *crtc = obj_to_crtc(obj);
1118 struct drm_crtc_state *crtc_state;
1119
1120 crtc_state = drm_atomic_get_crtc_state(state, crtc);
1121 if (IS_ERR(crtc_state)) {
1122 ret = PTR_ERR(crtc_state);
1123 break;
1124 }
1125
1126 if (async_flip) {
1127 ret = drm_atomic_crtc_get_property(crtc, crtc_state,
1128 prop, &old_val);
1129 ret = drm_atomic_check_prop_changes(ret, old_val, prop_value, prop);
1130 break;
1131 }
1132
1133 ret = drm_atomic_crtc_set_property(crtc,
1134 crtc_state, prop, prop_value);
1135 break;
1136 }
1137 case DRM_MODE_OBJECT_PLANE: {
1138 struct drm_plane *plane = obj_to_plane(obj);
1139 struct drm_plane_state *plane_state;
1140
1141 plane_state = drm_atomic_get_plane_state(state, plane);
1142 if (IS_ERR(plane_state)) {
1143 ret = PTR_ERR(plane_state);
1144 break;
1145 }
1146
1147 if (async_flip) {
1148 ret = drm_atomic_check_plane_changes(prop, plane, plane_state,
> 1149 obj, prop_value, old_val);
1150 if (ret)
1151 break;
1152 }
1153
1154 ret = drm_atomic_plane_set_property(plane,
1155 plane_state, file_priv,
1156 prop, prop_value);
1157 break;
1158 }
1159 default:
1160 drm_dbg_atomic(prop->dev, "[OBJECT:%d] has no properties\n", obj->id);
1161 ret = -EINVAL;
1162 break;
1163 }
1164
1165 drm_property_change_valid_put(prop, ref);
1166 return ret;
1167 }
1168
@@ -620,7 +620,7 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
return 0;
}
-static int
+int
drm_atomic_plane_get_property(struct drm_plane *plane,
const struct drm_plane_state *state,
struct drm_property *property, uint64_t *val)
@@ -683,6 +683,7 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
return 0;
}
+EXPORT_SYMBOL(drm_atomic_plane_get_property);
static int drm_atomic_set_writeback_fb_for_connector(
struct drm_connector_state *conn_state,
@@ -1026,18 +1027,54 @@ int drm_atomic_connector_commit_dpms(struct drm_atomic_state *state,
return ret;
}
-static int drm_atomic_check_prop_changes(int ret, uint64_t old_val, uint64_t prop_value,
+int drm_atomic_check_prop_changes(int ret, uint64_t old_val, uint64_t prop_value,
struct drm_property *prop)
{
if (ret != 0 || old_val != prop_value) {
drm_dbg_atomic(prop->dev,
- "[PROP:%d:%s] No prop can be changed during async flip\n",
+ "[PROP:%d:%s] This prop cannot be changed during async flip\n",
prop->base.id, prop->name);
return -EINVAL;
}
return 0;
}
+EXPORT_SYMBOL(drm_atomic_check_prop_changes);
+
+/* plane changes may have exceptions, so we have a special function for them */
+static int drm_atomic_check_plane_changes(struct drm_property *prop,
+ struct drm_plane *plane,
+ struct drm_plane_state *plane_state,
+ struct drm_mode_object *obj,
+ u64 prop_value, u64 old_val)
+{
+ struct drm_mode_config *config = &plane->dev->mode_config;
+ int ret;
+
+ if (plane->funcs->check_async_props)
+ return plane->funcs->check_async_props(prop, plane, plane_state,
+ obj, prop_value, old_val);
+
+ /*
+ * if you are trying to change something other than the FB ID, your
+ * change will be either rejected or ignored, so we can stop the check
+ * here
+ */
+ if (prop != config->prop_fb_id) {
+ ret = drm_atomic_plane_get_property(plane, plane_state,
+ prop, &old_val);
+ return drm_atomic_check_prop_changes(ret, old_val, prop_value, prop);
+ }
+
+ if (plane_state->plane->type != DRM_PLANE_TYPE_PRIMARY) {
+ drm_dbg_atomic(prop->dev,
+ "[OBJECT:%d] Only primary planes can be changed during async flip\n",
+ obj->id);
+ return -EINVAL;
+ }
+
+ return 0;
+}
int drm_atomic_set_property(struct drm_atomic_state *state,
struct drm_file *file_priv,
@@ -1100,7 +1137,6 @@ int drm_atomic_set_property(struct drm_atomic_state *state,
case DRM_MODE_OBJECT_PLANE: {
struct drm_plane *plane = obj_to_plane(obj);
struct drm_plane_state *plane_state;
- struct drm_mode_config *config = &plane->dev->mode_config;
plane_state = drm_atomic_get_plane_state(state, plane);
if (IS_ERR(plane_state)) {
@@ -1108,19 +1144,11 @@ int drm_atomic_set_property(struct drm_atomic_state *state,
break;
}
- if (async_flip && prop != config->prop_fb_id) {
- ret = drm_atomic_plane_get_property(plane, plane_state,
- prop, &old_val);
- ret = drm_atomic_check_prop_changes(ret, old_val, prop_value, prop);
- break;
- }
-
- if (async_flip && plane_state->plane->type != DRM_PLANE_TYPE_PRIMARY) {
- drm_dbg_atomic(prop->dev,
- "[OBJECT:%d] Only primary planes can be changed during async flip\n",
- obj->id);
- ret = -EINVAL;
- break;
+ if (async_flip) {
+ ret = drm_atomic_check_plane_changes(prop, plane, plane_state,
+ obj, prop_value, old_val);
+ if (ret)
+ break;
}
ret = drm_atomic_plane_set_property(plane,
@@ -29,6 +29,8 @@
#ifndef DRM_ATOMIC_UAPI_H_
#define DRM_ATOMIC_UAPI_H_
+#include <linux/types.h>
+
struct drm_crtc_state;
struct drm_display_mode;
struct drm_property_blob;
@@ -37,6 +39,9 @@ struct drm_crtc;
struct drm_connector_state;
struct dma_fence;
struct drm_framebuffer;
+struct drm_mode_object;
+struct drm_property;
+struct drm_plane;
int __must_check
drm_atomic_set_mode_for_crtc(struct drm_crtc_state *state,
@@ -53,4 +58,11 @@ int __must_check
drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state,
struct drm_crtc *crtc);
+int drm_atomic_plane_get_property(struct drm_plane *plane,
+ const struct drm_plane_state *state,
+ struct drm_property *property, uint64_t *val);
+
+int drm_atomic_check_prop_changes(int ret, uint64_t old_val, uint64_t prop_value,
+ struct drm_property *prop);
+
#endif
@@ -540,6 +540,11 @@ struct drm_plane_funcs {
*/
bool (*format_mod_supported)(struct drm_plane *plane, uint32_t format,
uint64_t modifier);
+
+ int (*check_async_props)(struct drm_property *prop, struct drm_plane *plane,
+ struct drm_plane_state *plane_state,
+ struct drm_mode_object *obj,
+ u64 prop_value, u64 old_val);
};
/**