Re: [PATCH] drm: allow IN_FENCE_FD and FB_DAMAGE_CLIPS to be changed with async commits
Great, thank you! Am Do., 11. Jan. 2024 um 19:05 Uhr schrieb André Almeida < andrealm...@igalia.com>: > Em 11/01/2024 14:59, Xaver Hugl escreveu: > > Am Do., 11. Jan. 2024 um 18:13 Uhr schrieb Simon Ser > > mailto:cont...@emersion.fr>>: > > > > Are we sure that all drivers handle these two props properly with > async > > page-flips? This is a new codepath not taken by the legacy uAPI. > > > > I've only tested on amdgpu so far. Afacs the other drivers that would > need > > testing / that support atomic and async pageflips are > > - i915 > > - noueveau (though atomic is disabled by default, so maybe it doesn't > > matter?) > > - vc4 > > - atmel-hlcdc > > > > The first two I can test, the latter I don't have the hardware for. I > > don't know if I can > > extensively test fb_damage_clips either / how I'd even know if it's > > being applied > > correctly, but in the worst case I'd expect the driver to not do the > > optimizations the > > property allows. > > > > As an alternative to this, would it be okay to expose a driver hook for > > optional > > driver-specific checks that drm_atomic_set_property can delegate to, and > > only > > allow this with the properties and hardware that's been tested? Then more > > properties (like cursor position changes on amdgpu) could be easily > > added later > > on too. > > I'm working on some mechanism to allow overlay planes on amdgpu, and I > think I can add your needs to it. I'll share in the mailing list when I > have something more concrete. >
Re: [PATCH] drm: allow IN_FENCE_FD and FB_DAMAGE_CLIPS to be changed with async commits
Am Do., 11. Jan. 2024 um 18:13 Uhr schrieb Simon Ser : > Are we sure that all drivers handle these two props properly with async > page-flips? This is a new codepath not taken by the legacy uAPI. > I've only tested on amdgpu so far. Afacs the other drivers that would need testing / that support atomic and async pageflips are - i915 - noueveau (though atomic is disabled by default, so maybe it doesn't matter?) - vc4 - atmel-hlcdc The first two I can test, the latter I don't have the hardware for. I don't know if I can extensively test fb_damage_clips either / how I'd even know if it's being applied correctly, but in the worst case I'd expect the driver to not do the optimizations the property allows. As an alternative to this, would it be okay to expose a driver hook for optional driver-specific checks that drm_atomic_set_property can delegate to, and only allow this with the properties and hardware that's been tested? Then more properties (like cursor position changes on amdgpu) could be easily added later on too.
Re: [PATCH] drm: allow IN_FENCE_FD and FB_DAMAGE_CLIPS to be changed with async commits
Em 11/01/2024 14:59, Xaver Hugl escreveu: Am Do., 11. Jan. 2024 um 18:13 Uhr schrieb Simon Ser mailto:cont...@emersion.fr>>: Are we sure that all drivers handle these two props properly with async page-flips? This is a new codepath not taken by the legacy uAPI. I've only tested on amdgpu so far. Afacs the other drivers that would need testing / that support atomic and async pageflips are - i915 - noueveau (though atomic is disabled by default, so maybe it doesn't matter?) - vc4 - atmel-hlcdc The first two I can test, the latter I don't have the hardware for. I don't know if I can extensively test fb_damage_clips either / how I'd even know if it's being applied correctly, but in the worst case I'd expect the driver to not do the optimizations the property allows. As an alternative to this, would it be okay to expose a driver hook for optional driver-specific checks that drm_atomic_set_property can delegate to, and only allow this with the properties and hardware that's been tested? Then more properties (like cursor position changes on amdgpu) could be easily added later on too. I'm working on some mechanism to allow overlay planes on amdgpu, and I think I can add your needs to it. I'll share in the mailing list when I have something more concrete.
Re: [PATCH] drm: allow IN_FENCE_FD and FB_DAMAGE_CLIPS to be changed with async commits
Are we sure that all drivers handle these two props properly with async page-flips? This is a new codepath not taken by the legacy uAPI. Style nit: the indentation is a bit off, the continuation lines don't align with the parenthesis.
Re: [PATCH] drm: allow IN_FENCE_FD and FB_DAMAGE_CLIPS to be changed with async commits
Hi Xaver,
Em 11/01/2024 13:56, Xaver Hugl escreveu:
Like with FB_ID, the driver never has to do bandwidth validation to use
these properties, so there's no reason not to allow them.
Signed-off-by: Xaver Hugl
Reviewed-by: André Almeida
---
drivers/gpu/drm/drm_atomic_uapi.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
b/drivers/gpu/drm/drm_atomic_uapi.c
index aee4a65d4959..06d476f5a746 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -1108,7 +1108,9 @@ int drm_atomic_set_property(struct drm_atomic_state
*state,
break;
}
- if (async_flip && prop != config->prop_fb_id) {
+ if (async_flip && prop != config->prop_fb_id &&
+ prop != config->prop_in_fence_fd &&
+ prop != config->prop_fb_damage_clips) {
ret = drm_atomic_plane_get_property(plane, plane_state,
prop, _val);
ret = drm_atomic_check_prop_changes(ret, old_val,
prop_value, prop);
