Re: [PATCH v6 6/6] drm/doc: Define KMS atomic state set
Hi Michel, On 8/17/23 12:37, Michel Dänzer wrote: On 8/15/23 20:57, André Almeida wrote: From: Pekka Paalanen Specify how the atomic state is maintained between userspace and kernel, plus the special case for async flips. Signed-off-by: Pekka Paalanen Signed-off-by: André Almeida [...] +An atomic commit with the flag DRM_MODE_PAGE_FLIP_ASYNC is allowed to +effectively change only the FB_ID property on any planes. No-operation changes +are ignored as always. [...] During the hackfest in Brno, it was mentioned that a commit which re-sets the same FB_ID could actually have an effect with VRR: It could trigger scanout of the next frame before vertical blank has reached its maximum duration. Some kind of mechanism is required for this in order to allow user space to perform low frame rate compensation. Xaver tested this hypothesis in a flipping the same fb in a VRR monitor and it worked as expected, so this shouldn't be a concern. Thanks, André
Re: [PATCH v6 6/6] drm/doc: Define KMS atomic state set
On Mon, 16 Oct 2023 12:52:32 +0200 André Almeida wrote: > Hi Michel, > > On 8/17/23 12:37, Michel Dänzer wrote: > > On 8/15/23 20:57, André Almeida wrote: > >> From: Pekka Paalanen > >> > >> Specify how the atomic state is maintained between userspace and > >> kernel, plus the special case for async flips. > >> > >> Signed-off-by: Pekka Paalanen > >> Signed-off-by: André Almeida > > [...] > > > >> +An atomic commit with the flag DRM_MODE_PAGE_FLIP_ASYNC is allowed to > >> +effectively change only the FB_ID property on any planes. No-operation > >> changes > >> +are ignored as always. [...] > > During the hackfest in Brno, it was mentioned that a commit which re-sets > > the same FB_ID could actually have an effect with VRR: It could trigger > > scanout of the next frame before vertical blank has reached its maximum > > duration. Some kind of mechanism is required for this in order to allow > > user space to perform low frame rate compensation. > > > Xaver tested this hypothesis in a flipping the same fb in a VRR monitor > and it worked as expected, so this shouldn't be a concern. Right, so it must have some effect. It cannot be simply ignored like in the proposed doc wording. Do we special-case re-setting the same FB_ID as "not a no-op" or "not ignored" or some other way? Thanks, pq pgpjtp9BAtZ4A.pgp Description: OpenPGP digital signature
Re: [PATCH v6 6/6] drm/doc: Define KMS atomic state set
Hi Pekka, On 10/16/23 14:18, Pekka Paalanen wrote: On Mon, 16 Oct 2023 12:52:32 +0200 André Almeida wrote: Hi Michel, On 8/17/23 12:37, Michel Dänzer wrote: On 8/15/23 20:57, André Almeida wrote: From: Pekka Paalanen Specify how the atomic state is maintained between userspace and kernel, plus the special case for async flips. Signed-off-by: Pekka Paalanen Signed-off-by: André Almeida [...] +An atomic commit with the flag DRM_MODE_PAGE_FLIP_ASYNC is allowed to +effectively change only the FB_ID property on any planes. No-operation changes +are ignored as always. [...] During the hackfest in Brno, it was mentioned that a commit which re-sets the same FB_ID could actually have an effect with VRR: It could trigger scanout of the next frame before vertical blank has reached its maximum duration. Some kind of mechanism is required for this in order to allow user space to perform low frame rate compensation. Xaver tested this hypothesis in a flipping the same fb in a VRR monitor and it worked as expected, so this shouldn't be a concern. Right, so it must have some effect. It cannot be simply ignored like in the proposed doc wording. Do we special-case re-setting the same FB_ID as "not a no-op" or "not ignored" or some other way? There's an effect in the refresh rate, the image won't change but it will report that a flip had happened asynchronously so the reported framerate will be increased. Maybe an additional wording could be like: Flipping to the same FB_ID will result in a immediate flip as if it was changing to a different one, with no effect on the image but effecting the reported frame rate. Thanks, pq
Re: [PATCH v6 6/6] drm/doc: Define KMS atomic state set
On Mon, 16 Oct 2023 15:42:16 +0200 André Almeida wrote: > Hi Pekka, > > On 10/16/23 14:18, Pekka Paalanen wrote: > > On Mon, 16 Oct 2023 12:52:32 +0200 > > André Almeida wrote: > > > >> Hi Michel, > >> > >> On 8/17/23 12:37, Michel Dänzer wrote: > >>> On 8/15/23 20:57, André Almeida wrote: > From: Pekka Paalanen > > Specify how the atomic state is maintained between userspace and > kernel, plus the special case for async flips. > > Signed-off-by: Pekka Paalanen > Signed-off-by: André Almeida > >>> [...] > >>> > +An atomic commit with the flag DRM_MODE_PAGE_FLIP_ASYNC is allowed to > +effectively change only the FB_ID property on any planes. No-operation > changes > +are ignored as always. [...] > >>> During the hackfest in Brno, it was mentioned that a commit which re-sets > >>> the same FB_ID could actually have an effect with VRR: It could trigger > >>> scanout of the next frame before vertical blank has reached its maximum > >>> duration. Some kind of mechanism is required for this in order to allow > >>> user space to perform low frame rate compensation. > >>> > >> Xaver tested this hypothesis in a flipping the same fb in a VRR monitor > >> and it worked as expected, so this shouldn't be a concern. > > Right, so it must have some effect. It cannot be simply ignored like in > > the proposed doc wording. Do we special-case re-setting the same FB_ID > > as "not a no-op" or "not ignored" or some other way? > There's an effect in the refresh rate, the image won't change but it > will report that a flip had happened asynchronously so the reported > framerate will be increased. Maybe an additional wording could be like: > > Flipping to the same FB_ID will result in a immediate flip as if it was > changing to a different one, with no effect on the image but effecting > the reported frame rate. Re-setting FB_ID to its current value is a special case regardless of PAGE_FLIP_ASYNC, is it not? So it should be called out somewhere that applies regardless of PAGE_FLIP_ASYNC. Maybe to the end of the earlier paragraph: > +The changes recorded in an atomic commit apply on top the current KMS state > in > +the kernel. Hence, the complete new KMS state is the complete old KMS state > with > +the committed property settings done on top. The kernel will try to avoid > +no-operation changes, so it is safe for userspace to send redundant property > +settings. However, not every situation allows for no-op changes, due to the > +need to acquire locks for some attributes. Userspace needs to be aware that > some > +redundant information might result in oversynchronization issues. > No-operation > +changes do not count towards actually needed changes, e.g. setting MODE_ID > to a > +different blob with identical contents as the current KMS state shall not be > a > +modeset on its own. +As a special exception for VRR needs, explicitly setting FB_ID to its +current value is not a no-op. Would that work? I'd like to try to avoid being more specific about what it does exactly, because that's not the topic here. Such things can be documented with the property itself. This is a summary of what is or is not a no-op or a modeset. Thanks, pq pgpYkXWg9gQWt.pgp Description: OpenPGP digital signature
Re: [PATCH v6 6/6] drm/doc: Define KMS atomic state set
On 10/16/23 16:52, Pekka Paalanen wrote: On Mon, 16 Oct 2023 15:42:16 +0200 André Almeida wrote: Hi Pekka, On 10/16/23 14:18, Pekka Paalanen wrote: On Mon, 16 Oct 2023 12:52:32 +0200 André Almeida wrote: Hi Michel, On 8/17/23 12:37, Michel Dänzer wrote: On 8/15/23 20:57, André Almeida wrote: From: Pekka Paalanen Specify how the atomic state is maintained between userspace and kernel, plus the special case for async flips. Signed-off-by: Pekka Paalanen Signed-off-by: André Almeida [...] +An atomic commit with the flag DRM_MODE_PAGE_FLIP_ASYNC is allowed to +effectively change only the FB_ID property on any planes. No-operation changes +are ignored as always. [...] During the hackfest in Brno, it was mentioned that a commit which re-sets the same FB_ID could actually have an effect with VRR: It could trigger scanout of the next frame before vertical blank has reached its maximum duration. Some kind of mechanism is required for this in order to allow user space to perform low frame rate compensation. Xaver tested this hypothesis in a flipping the same fb in a VRR monitor and it worked as expected, so this shouldn't be a concern. Right, so it must have some effect. It cannot be simply ignored like in the proposed doc wording. Do we special-case re-setting the same FB_ID as "not a no-op" or "not ignored" or some other way? There's an effect in the refresh rate, the image won't change but it will report that a flip had happened asynchronously so the reported framerate will be increased. Maybe an additional wording could be like: Flipping to the same FB_ID will result in a immediate flip as if it was changing to a different one, with no effect on the image but effecting the reported frame rate. Re-setting FB_ID to its current value is a special case regardless of PAGE_FLIP_ASYNC, is it not? So it should be called out somewhere that applies regardless of PAGE_FLIP_ASYNC. Maybe to the end of the earlier paragraph: +The changes recorded in an atomic commit apply on top the current KMS state in +the kernel. Hence, the complete new KMS state is the complete old KMS state with +the committed property settings done on top. The kernel will try to avoid +no-operation changes, so it is safe for userspace to send redundant property +settings. However, not every situation allows for no-op changes, due to the +need to acquire locks for some attributes. Userspace needs to be aware that some +redundant information might result in oversynchronization issues. No-operation +changes do not count towards actually needed changes, e.g. setting MODE_ID to a +different blob with identical contents as the current KMS state shall not be a +modeset on its own. +As a special exception for VRR needs, explicitly setting FB_ID to its +current value is not a no-op. Would that work? I liked this suggestion, thanks! I'll wrap up a v7 I'd like to try to avoid being more specific about what it does exactly, because that's not the topic here. Such things can be documented with the property itself. This is a summary of what is or is not a no-op or a modeset. Thanks, pq
Re: [PATCH v6 6/6] drm/doc: Define KMS atomic state set
On Mon, Oct 16, 2023 at 05:52:22PM +0300, Pekka Paalanen wrote: > On Mon, 16 Oct 2023 15:42:16 +0200 > André Almeida wrote: > > > Hi Pekka, > > > > On 10/16/23 14:18, Pekka Paalanen wrote: > > > On Mon, 16 Oct 2023 12:52:32 +0200 > > > André Almeida wrote: > > > > > >> Hi Michel, > > >> > > >> On 8/17/23 12:37, Michel Dänzer wrote: > > >>> On 8/15/23 20:57, André Almeida wrote: > > From: Pekka Paalanen > > > > Specify how the atomic state is maintained between userspace and > > kernel, plus the special case for async flips. > > > > Signed-off-by: Pekka Paalanen > > Signed-off-by: André Almeida > > >>> [...] > > >>> > > +An atomic commit with the flag DRM_MODE_PAGE_FLIP_ASYNC is allowed to > > +effectively change only the FB_ID property on any planes. > > No-operation changes > > +are ignored as always. [...] > > >>> During the hackfest in Brno, it was mentioned that a commit which > > >>> re-sets the same FB_ID could actually have an effect with VRR: It could > > >>> trigger scanout of the next frame before vertical blank has reached its > > >>> maximum duration. Some kind of mechanism is required for this in order > > >>> to allow user space to perform low frame rate compensation. > > >>> > > >> Xaver tested this hypothesis in a flipping the same fb in a VRR monitor > > >> and it worked as expected, so this shouldn't be a concern. > > > Right, so it must have some effect. It cannot be simply ignored like in > > > the proposed doc wording. Do we special-case re-setting the same FB_ID > > > as "not a no-op" or "not ignored" or some other way? > > There's an effect in the refresh rate, the image won't change but it > > will report that a flip had happened asynchronously so the reported > > framerate will be increased. Maybe an additional wording could be like: > > > > Flipping to the same FB_ID will result in a immediate flip as if it was > > changing to a different one, with no effect on the image but effecting > > the reported frame rate. > > Re-setting FB_ID to its current value is a special case regardless of > PAGE_FLIP_ASYNC, is it not? No. The rule has so far been that all side effects are observed even if you flip to the same fb. And that is one of my annoyances with this proposal. The rules will now be different for async flips vs. everything else. The other issues (mainly relating to hardware where not all planes support async flips) I've already highlighted in some earlier mail. -- Ville Syrjälä Intel
Re: [PATCH v1] dynamic_debug: add support for logs destination
czw., 12 paź 2023 o 20:48 napisał(a): > > > If you want the kernel to keep separate flight recorders I guess we could > > add that, but I don't think it currently exists for the dyndbg stuff at > > least. Maybe a flight recorder v2 feature, once the basics are in. > > > > dyndbg has +pwrites to syslog > +T would separately independently write the same to global trace > > This would allow graceful switchover to tracefs, > without removing logging from dmesg, where most folks > (and any monitor tools) would expect it. > > Lukas (iiuc) wants to steer each site to just 1 destination. > Or maybe (in addition to +p > syslog) one trace destination, > either global via events, or a separate tracebuf > > Im ambivalent, but thinking the smooth rollover from syslog to trace > might be worth having to ease migration / weaning off syslog. > > And we have a 4 byte hole in struct _ddebug we could just use. I'm glad you brought that up. This means we can leave class_id field untouched, have separate +T in flags (for consistency) and dst_id can be easily 8 bits wide. Also can you point me to the latest version of writing debug logs to trace events (+T option). I would like to base trace instances work on that because both are closely related. > Unless the align 8 is optional on 32-bits, I verified with "gcc -g -m32 ..." that the align(8) is honored on 32 bits. > I think we're never gonna close the hole anywhere. > :) > is align 8 a generic expression of an architectural simplifying constraint ? > or a need for 1-7 ptr offsets ? > > > > > > > That's my idea of it. It is interesting to see how far the requirements > > > can be reasonably realised. > > > > I think aside from the "make it available directly to unpriviledged > > userspace" everything sounds reasonable and doable. > > > > More on the process side of things, I think Jim is very much looking for > > acks and tested-by by people who are interested in better drm logging > > infra. That should help that things are moving in a direction that's > > actually useful, even when it's not yet entirely complete. > > > > yes, please. Now posted at > > https://lore.kernel.org/lkml/20231012172137.3286566-1-jim.cro...@gmail.com/T/#t > > Lukas, I managed to miss your email in the send phase. > please consider yourself a direct recipient :-) > > thanks everyone > > > Cheers, Sima > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch
Re: [PATCH v1] dynamic_debug: add support for logs destination
On Thu, 12 Oct 2023 11:53:52 +0200 Daniel Vetter wrote: > > You said that turning the kernel ring buffer contents into strings is a > > very heavy operation, so it is not possible to push this scope > > separation to userspace, right? > > I think it's the kernel that does the formatting, but honestly not sure > how this works with perf traces. Might be that it's actually userspace > doing the formatting later on so that it doesn't incur the overhead while > recording. perf and trace-cmd do the formatting in user space via libtraceevent: git://git.kernel.org/pub/scm/libs/libtrace/libtraceevent.git It reads the format files of the events: /sys/kernel/tracing/events/*/*/format and uses that to read the raw data saved from the kernel into human readable output. Note, this means that addresses coming from kernel trace events are not hashed! -- Steve
[ANNOUNCE] weston 12.0.91
Hi all, This is the alpha release for Weston 13.0.0. Notes for packagers: - backend-vnc requires Neat VNC 0.7.0 - launcher-logind has been removed Thanks to all contributors! Full history commit below: Alexandros Frantzis (3): xwayland: Allow shells to make xwayland surfaces fullscreen xwayland: Notify the shell when a window drops the fullscreen state desktop-shell: Don't process surfaces under destruction during output resize Brendan King (1): xwayland: fix segfault when running x11perf Christopher Obbard (2): libweston: Decouple dbus helper to public namespace libweston: Split dbus support into seperate build option Daniel Stone (153): CI: Upgrade ci-templates version CI: Parameterise LLVM version CI: Rename debian jobs to debian-lts CI: Force pip to install packages CI: Bump virtme snapshot version CI: Upgrade Mesa and kernel CI: Add Debian bookworm jobs CI: Require wayland 1.22 for wl_client_add_destroy_late_listener frontend: Move path into weston_process frontend: Remove process_info frontend: Rename weston_process to wet_process frontend: Inline wet_watch_process frontend: Rename weston_client_start to wet_client_start frontend: Add data argument to wet_process cleanup frontend: Allow NULL wet_process cleanup handler frontend: Allocate new wet_process for Xwayland frontend: Make wet_client_launch allocate new wet_process frontend: Inline process_handle_sigchld frontend: Factor out wet_process_destroy xwayland: Only pass Xwayland wl_client to libweston tests/xwayland: Ensure $DISPLAY is correctly set xwayland: Remove process exit status from libweston API xwayland: Rename destroy_listener to be more explicit xwayland: Add client destroy listener frontend: Return user-data pointer from Xwayland init frontend: Explicitly destroy Xwayland from frontend code frontend: Clean up wet_processes on exit libweston: Add weston_compositor.shutting_down backend-drm: Use weston_compositor.shutting_down desktop-shell: Don't restart client on exit text-backend: Don't restart client on exit backend-drm: Don't leak writeback-format property blob tests/drm: Fix leaks in drm-writeback-screenshot-test CI: Enable ASan memory-leak checking surface: Convert a couple of ints to bools surface: Move presentation-feedback discard to commit surface: Start tracking weston_surface_status surface: Only rebuild paint node regions when necessary surface: Convert a couple of bools to dirty flags surface: Inline buffer-size calculation to attach surface: Pass weston_surface_state into attach build: Switch join_paths(foo, bar) to foo / bar build: Run tests with leak-sanitizer suppressions CI: Remove per-test-asan wrapper weston_surface: Add map and unmap signals desktop-shell: shell_surfaces always have a layer desktop-shell: Actually dirty surface regions when moving desktop-shell: Don't damage unmapped views desktop-shell: Make view mapping more consistent desktop-shell: Centralise view mapping for shell_surfaces weston-desktop: Match desktop-shell view mapping semantics libweston: Add weston_view::map_signal desktop-shell: Use weston_view_move_to_layer() for fullscreen switching desktop-shell: Use weston_view_move_to_layer() for fullscreen fades desktop-shell: Use weston_view_move_to_layer() for per-view unfade desktop-shell: Create fade-out views at destroy time desktop-shell: Use weston_view_move_to_layer() for fullscreen background desktop-shell: Use weston_view_move_to_layer() for static views desktop-shell: Use weston_view_to_layer() for lock surface desktop-shell: Be more precise with rotation damage kiosk-shell: Use weston_view_move_to_layer() for activation kiosk-shell: Use weston_view_move_to_layer() for background kiosk-shell: Use weston_view_move_to_layer() for view activation fullscreen-shell: Use weston_view_move_to_layer() weston-test-desktop-shell: Use weston_view_move_to_layer() build: Avoid Meson warning for run_command() without check tests: Remove single case for device destroy test tests: Pass wet_testsuite_data to test runs tests: Track weston_outputs in weston-test plugin tests: Add client<->compositor breakpoint support tests: Add paint-node test tests: Initialise breakpoint list for all test types desktop-shell: Remove unused fullscreen transform surface: Only rebuild surface size where necessary surface: Replace newly_attached with weston_surface_status surface: Replace viewport.changed with weston_surface_status surface: Add comments to weston_surface_status member surface: Add position dirty member
Re: [PATCH v6 6/6] drm/doc: Define KMS atomic state set
On Monday, October 16th, 2023 at 17:10, Ville Syrjälä wrote: > On Mon, Oct 16, 2023 at 05:52:22PM +0300, Pekka Paalanen wrote: > > > On Mon, 16 Oct 2023 15:42:16 +0200 > > André Almeida andrealm...@igalia.com wrote: > > > > > Hi Pekka, > > > > > > On 10/16/23 14:18, Pekka Paalanen wrote: > > > > > > > On Mon, 16 Oct 2023 12:52:32 +0200 > > > > André Almeida andrealm...@igalia.com wrote: > > > > > > > > > Hi Michel, > > > > > > > > > > On 8/17/23 12:37, Michel Dänzer wrote: > > > > > > > > > > > On 8/15/23 20:57, André Almeida wrote: > > > > > > > > > > > > > From: Pekka Paalanen pekka.paala...@collabora.com > > > > > > > > > > > > > > Specify how the atomic state is maintained between userspace and > > > > > > > kernel, plus the special case for async flips. > > > > > > > > > > > > > > Signed-off-by: Pekka Paalanen pekka.paala...@collabora.com > > > > > > > Signed-off-by: André Almeida andrealm...@igalia.com > > > > > > > [...] > > > > > > > > > > > > > +An atomic commit with the flag DRM_MODE_PAGE_FLIP_ASYNC is > > > > > > > allowed to > > > > > > > +effectively change only the FB_ID property on any planes. > > > > > > > No-operation changes > > > > > > > +are ignored as always. [...] > > > > > > > During the hackfest in Brno, it was mentioned that a commit which > > > > > > > re-sets the same FB_ID could actually have an effect with VRR: It > > > > > > > could trigger scanout of the next frame before vertical blank has > > > > > > > reached its maximum duration. Some kind of mechanism is required > > > > > > > for this in order to allow user space to perform low frame rate > > > > > > > compensation. > > > > > > > > > > Xaver tested this hypothesis in a flipping the same fb in a VRR > > > > > monitor > > > > > and it worked as expected, so this shouldn't be a concern. > > > > > Right, so it must have some effect. It cannot be simply ignored like > > > > > in > > > > > the proposed doc wording. Do we special-case re-setting the same FB_ID > > > > > as "not a no-op" or "not ignored" or some other way? > > > > > There's an effect in the refresh rate, the image won't change but it > > > > > will report that a flip had happened asynchronously so the reported > > > > > framerate will be increased. Maybe an additional wording could be > > > > > like: > > > > > > Flipping to the same FB_ID will result in a immediate flip as if it was > > > changing to a different one, with no effect on the image but effecting > > > the reported frame rate. > > > > Re-setting FB_ID to its current value is a special case regardless of > > PAGE_FLIP_ASYNC, is it not? > > No. The rule has so far been that all side effects are observed > even if you flip to the same fb. And that is one of my annoyances > with this proposal. The rules will now be different for async flips > vs. everything else. Well with the patches the async page-flip case is exactly the same as the non-async page-flip case. In both cases, if a FB_ID is included in an atomic commit then the side effects are triggered even if the property value didn't change. The rules are the same for everything.
Re: [Freedreno] [PATCH RFC v6 07/10] drm/atomic: Loosen FB atomic checks
On 9/24/2023 3:23 AM, Dmitry Baryshkov wrote:
On 22/09/2023 20:49, Jessica Zhang wrote:
On 8/29/2023 1:22 AM, Pekka Paalanen wrote:
On Mon, 28 Aug 2023 17:05:13 -0700
Jessica Zhang wrote:
Loosen the requirements for atomic and legacy commit so that, in cases
where pixel_source != FB, the commit can still go through.
This includes adding framebuffer NULL checks in other areas to
account for
FB being NULL when non-FB pixel sources are enabled.
To disable a plane, the pixel_source must be NONE or the FB must be
NULL
if pixel_source == FB.
Signed-off-by: Jessica Zhang
---
drivers/gpu/drm/drm_atomic.c | 20 +++-
drivers/gpu/drm/drm_atomic_helper.c | 36
include/drm/drm_atomic_helper.h | 4 ++--
include/drm/drm_plane.h | 29
+
4 files changed, 62 insertions(+), 27 deletions(-)
...
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index a58f84b6bd5e..4c5b7bcdb25c 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -992,6 +992,35 @@ static inline struct drm_plane
*drm_plane_find(struct drm_device *dev,
#define drm_for_each_plane(plane, dev) \
list_for_each_entry(plane, &(dev)->mode_config.plane_list, head)
+/**
+ * drm_plane_solid_fill_enabled - Check if solid fill is enabled on
plane
+ * @state: plane state
+ *
+ * Returns:
+ * Whether the plane has been assigned a solid_fill_blob
+ */
+static inline bool drm_plane_solid_fill_enabled(struct
drm_plane_state *state)
+{
+ if (!state)
+ return false;
+ return state->pixel_source == DRM_PLANE_PIXEL_SOURCE_SOLID_FILL
&& state->solid_fill_blob;
+}
+
+static inline bool drm_plane_has_visible_data(const struct
drm_plane_state *state)
+{
+ switch (state->pixel_source) {
+ case DRM_PLANE_PIXEL_SOURCE_NONE:
+ return false;
+ case DRM_PLANE_PIXEL_SOURCE_SOLID_FILL:
+ return state->solid_fill_blob != NULL;
This reminds me, new UAPI docs did not say what the requirements are for
choosing solid fill pixel source. Is the atomic commit rejected if
pixel source is solid fill, but solid_fill property has no blob?
Hi Pekka,
Yes, if pixel_source is solid_fill and the solid_fill property blob
isn't set, the atomic commit should throw an error.
Will document this in the UAPI.
I don't see a corresponding error check in atomic_check() functions.
Could you please check that there is one, as you are updating the uAPI.
Hi Dmitry,
Sorry for the late response.
drm_plane_has_visible_data() is being called from
drm_atomic_plane_check() which is called from drm_atomic_commit() (via
drm_atomic_check_only()).
It's also called within the atomic_check() callstack in
drm_atomic_helper_check_plane_state(), though that check will set
plane.visible to false and return 0.
Would it be better to have a more explicit `if (source == solid_fill &&
!plane->solid_fill_blob) then return -EINVAL` check in atomic_check()?
Thanks,
Jessica Zhang
Thanks,
Jessica Zhang
This should be doc'd.
Thanks,
pq
+ case DRM_PLANE_PIXEL_SOURCE_FB:
+ default:
+ WARN_ON(state->pixel_source != DRM_PLANE_PIXEL_SOURCE_FB);
+ }
+
+ return state->fb != NULL;
+}
+
bool drm_any_plane_has_format(struct drm_device *dev,
u32 format, u64 modifier);
--
With best wishes
Dmitry
