* [PATCH] drm: vc4: adapt to new behaviour of drm_crtc.c [not found] <CGME20170201093608eucas1p27614e4c715b5446a7ea13fa9edf3b33d@eucas1p2.samsung.com> @ 2017-02-01 9:35 ` Andrzej Pietrasiewicz 2017-02-08 0:33 ` Eric Anholt 0 siblings, 1 reply; 5+ messages in thread From: Andrzej Pietrasiewicz @ 2017-02-01 9:35 UTC (permalink / raw) To: dri-devel Cc: Bartlomiej Zolnierkiewicz, Andrzej Pietrasiewicz, Marek Szyprowski When drm_crtc_init_with_planes() was orignally added (in drm_crtc.c, e13161af80c185ecd8dc4641d0f5df58f9e3e0af drm: Add drm_crtc_init_with_planes() (v2)), it only checked for "primary" being non-null. If that was the case, it modified primary->possible_crtcs. Then, when support for cursor planes was added (fc1d3e44ef7c1db93384150fdbf8948dcf949f15 drm: Allow drivers to register cursor planes with crtc), the same behaviour was implemented for cursor planes. vc4_plane_init() since its inception has passed 0xff as "possible_crtcs" parameter to drm_universal_plane_init(). With a change in drm_crtc.c (7abc7d47510c75dd984380ebf819616e574c9604 drm: don't override possible_crtcs for primary/cursor planes) passing 0xff results in primary's and cursor's possible_crtcs set to 0xff. Consequently, these planes (incorrectly) match many crtcs. This patch gives drm_ctc_init_with_planes() a chance to properly set possible_crtcs for primary and cursor planes. Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@samsung.com> --- drivers/gpu/drm/vc4/vc4_plane.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index 881bf48..686cdd3 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -858,7 +858,7 @@ struct drm_plane *vc4_plane_init(struct drm_device *dev, } } plane = &vc4_plane->base; - ret = drm_universal_plane_init(dev, plane, 0xff, + ret = drm_universal_plane_init(dev, plane, 0, &vc4_plane_funcs, formats, num_formats, type, NULL); -- 1.9.1 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drm: vc4: adapt to new behaviour of drm_crtc.c 2017-02-01 9:35 ` [PATCH] drm: vc4: adapt to new behaviour of drm_crtc.c Andrzej Pietrasiewicz @ 2017-02-08 0:33 ` Eric Anholt 2017-02-08 7:13 ` Daniel Vetter 2017-02-08 8:29 ` Andrzej Pietrasiewicz 0 siblings, 2 replies; 5+ messages in thread From: Eric Anholt @ 2017-02-08 0:33 UTC (permalink / raw) To: dri-devel Cc: Andrzej Pietrasiewicz, Bartlomiej Zolnierkiewicz, Marek Szyprowski [-- Attachment #1.1: Type: text/plain, Size: 1951 bytes --] Andrzej Pietrasiewicz <andrzej.p@samsung.com> writes: > When drm_crtc_init_with_planes() was orignally added > (in drm_crtc.c, e13161af80c185ecd8dc4641d0f5df58f9e3e0af > drm: Add drm_crtc_init_with_planes() (v2)), it only checked for "primary" > being non-null. If that was the case, it modified primary->possible_crtcs. > > Then, when support for cursor planes was added > (fc1d3e44ef7c1db93384150fdbf8948dcf949f15 drm: Allow drivers to register > cursor planes with crtc), the same behaviour was implemented for cursor > planes. > > vc4_plane_init() since its inception has passed 0xff as "possible_crtcs" > parameter to drm_universal_plane_init(). With a change in drm_crtc.c > (7abc7d47510c75dd984380ebf819616e574c9604 drm: don't override > possible_crtcs for primary/cursor planes) passing 0xff results in primary's > and cursor's possible_crtcs set to 0xff. Consequently, these planes > (incorrectly) match many crtcs. This patch gives drm_ctc_init_with_planes() > a chance to properly set possible_crtcs for primary and cursor planes. > > Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@samsung.com> Thanks for the detailed explanation! I think the cursor's possible_crtcs is getting set in vc4_crtc.c (since we don't pass the cursor into drm_crtc_init_with_planes()). I was also skeptical that there would be an actual visible bug, since our planes aren't really attached to the CRTCs other than the CRTC putting the plane's dlist entry into their dlist. However, I think if you stole CRTC 1's primary plane for CRTC 0 instead of CRTC 0's primary, the primary plane would appear on top of the overlays and cursor of CRTC 0. 7abc7d47510c75dd984380ebf819616e574c9604 appears only in 4.10-rc*. I think I want to add to the commit message: Fixes: 7abc7d47510c ("drm: don't override possible_crtcs for primary/cursor planes") and push this to drm-misc-fixes. drm-misc maintainers, can you confirm that this is the right way to handle it? [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm: vc4: adapt to new behaviour of drm_crtc.c 2017-02-08 0:33 ` Eric Anholt @ 2017-02-08 7:13 ` Daniel Vetter 2017-02-08 8:29 ` Andrzej Pietrasiewicz 1 sibling, 0 replies; 5+ messages in thread From: Daniel Vetter @ 2017-02-08 7:13 UTC (permalink / raw) To: Eric Anholt Cc: Andrzej Pietrasiewicz, Marek Szyprowski, dri-devel, Bartlomiej Zolnierkiewicz On Wed, Feb 8, 2017 at 1:33 AM, Eric Anholt <eric@anholt.net> wrote: > Andrzej Pietrasiewicz <andrzej.p@samsung.com> writes: > >> When drm_crtc_init_with_planes() was orignally added >> (in drm_crtc.c, e13161af80c185ecd8dc4641d0f5df58f9e3e0af >> drm: Add drm_crtc_init_with_planes() (v2)), it only checked for "primary" >> being non-null. If that was the case, it modified primary->possible_crtcs. >> >> Then, when support for cursor planes was added >> (fc1d3e44ef7c1db93384150fdbf8948dcf949f15 drm: Allow drivers to register >> cursor planes with crtc), the same behaviour was implemented for cursor >> planes. >> >> vc4_plane_init() since its inception has passed 0xff as "possible_crtcs" >> parameter to drm_universal_plane_init(). With a change in drm_crtc.c >> (7abc7d47510c75dd984380ebf819616e574c9604 drm: don't override >> possible_crtcs for primary/cursor planes) passing 0xff results in primary's >> and cursor's possible_crtcs set to 0xff. Consequently, these planes >> (incorrectly) match many crtcs. This patch gives drm_ctc_init_with_planes() >> a chance to properly set possible_crtcs for primary and cursor planes. >> >> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@samsung.com> > > Thanks for the detailed explanation! > > I think the cursor's possible_crtcs is getting set in vc4_crtc.c (since > we don't pass the cursor into drm_crtc_init_with_planes()). I was also > skeptical that there would be an actual visible bug, since our planes > aren't really attached to the CRTCs other than the CRTC putting the > plane's dlist entry into their dlist. However, I think if you stole > CRTC 1's primary plane for CRTC 0 instead of CRTC 0's primary, the > primary plane would appear on top of the overlays and cursor of CRTC 0. > > 7abc7d47510c75dd984380ebf819616e574c9604 appears only in 4.10-rc*. I > think I want to add to the commit message: > > Fixes: 7abc7d47510c ("drm: don't override possible_crtcs for primary/cursor planes") > > and push this to drm-misc-fixes. drm-misc maintainers, can you confirm > that this is the right way to handle it? Yeah, I'll try to get the pr out still so it'll hit 4.10. Otherwise it's going to be for 4.11. For safety maybe add a cc: stable # 4.10 -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm: vc4: adapt to new behaviour of drm_crtc.c 2017-02-08 0:33 ` Eric Anholt 2017-02-08 7:13 ` Daniel Vetter @ 2017-02-08 8:29 ` Andrzej Pietrasiewicz 2017-02-08 22:20 ` Eric Anholt 1 sibling, 1 reply; 5+ messages in thread From: Andrzej Pietrasiewicz @ 2017-02-08 8:29 UTC (permalink / raw) To: Eric Anholt, dri-devel; +Cc: Bartlomiej Zolnierkiewicz, Marek Szyprowski Hi Eric, W dniu 08.02.2017 o 01:33, Eric Anholt pisze: > Andrzej Pietrasiewicz <andrzej.p@samsung.com> writes: > <snip> > > Thanks for the detailed explanation! > > I think the cursor's possible_crtcs is getting set in vc4_crtc.c (since > we don't pass the cursor into drm_crtc_init_with_planes()). You are right. Shall I submit a v2 patch with a corrected commit message? Andrzej _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm: vc4: adapt to new behaviour of drm_crtc.c 2017-02-08 8:29 ` Andrzej Pietrasiewicz @ 2017-02-08 22:20 ` Eric Anholt 0 siblings, 0 replies; 5+ messages in thread From: Eric Anholt @ 2017-02-08 22:20 UTC (permalink / raw) To: Andrzej Pietrasiewicz, dri-devel Cc: Bartlomiej Zolnierkiewicz, Marek Szyprowski [-- Attachment #1.1: Type: text/plain, Size: 574 bytes --] Andrzej Pietrasiewicz <andrzej.p@samsung.com> writes: > Hi Eric, > > W dniu 08.02.2017 o 01:33, Eric Anholt pisze: >> Andrzej Pietrasiewicz <andrzej.p@samsung.com> writes: >> > > <snip> > >> >> Thanks for the detailed explanation! >> >> I think the cursor's possible_crtcs is getting set in vc4_crtc.c (since >> we don't pass the cursor into drm_crtc_init_with_planes()). > > You are right. Shall I submit a v2 patch with a corrected commit message? I've gone ahead and updated the commit message and pushed to -fixes, so we can hopefully make it in the next PR. Thanks! [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-02-08 22:20 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CGME20170201093608eucas1p27614e4c715b5446a7ea13fa9edf3b33d@eucas1p2.samsung.com> 2017-02-01 9:35 ` [PATCH] drm: vc4: adapt to new behaviour of drm_crtc.c Andrzej Pietrasiewicz 2017-02-08 0:33 ` Eric Anholt 2017-02-08 7:13 ` Daniel Vetter 2017-02-08 8:29 ` Andrzej Pietrasiewicz 2017-02-08 22:20 ` Eric Anholt
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.