| Message ID | 20240116045159.1015510-2-andrealmeid@igalia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/atomic: Allow drivers to write their own plane check for async | expand |
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
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index aee4a65d4959..6d5b9fec90c7 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -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, diff --git a/include/drm/drm_atomic_uapi.h b/include/drm/drm_atomic_uapi.h index 4c6d39d7bdb2..d65fa8fbbca0 100644 --- a/include/drm/drm_atomic_uapi.h +++ b/include/drm/drm_atomic_uapi.h @@ -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 diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h index c6565a6f9324..73dac8d76831 100644 --- a/include/drm/drm_plane.h +++ b/include/drm/drm_plane.h @@ -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); }; /**
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(-)