Message ID | 1459784845-13517-1-git-send-email-marius.c.vlad@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Hi Marius, Thanks for this. I have the same patch on a branch I haven't send yet (oops). In my patch I implemented this by setting src_* to 0 if fb_id == 0. I'm not sure what makes more sense but anyway : Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> On 04/04/16 16:47, Marius Vlad wrote: > igt_atomic_prepare_plane_commit() assumes that the framebuffer is always > set-up. > > Signed-off-by: Marius Vlad <marius.c.vlad@intel.com> > --- > lib/igt_kms.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/igt_kms.c b/lib/igt_kms.c > index 82257a6..30c5b7e 100644 > --- a/lib/igt_kms.c > +++ b/lib/igt_kms.c > @@ -1585,7 +1585,7 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_output_t *output, > igt_atomic_populate_plane_req(req, plane, IGT_PLANE_FB_ID, fb_id); > } > > - if (plane->position_changed || plane->size_changed) { > + if ((plane->position_changed || plane->size_changed) && fb_id != 0) { > uint32_t src_x = IGT_FIXED(plane->fb->src_x, 0); /* src_x */ > uint32_t src_y = IGT_FIXED(plane->fb->src_y, 0); /* src_y */ > uint32_t src_w = IGT_FIXED(plane->fb->src_w, 0); /* src_w */
On Mon, Apr 04, 2016 at 06:47:25PM +0300, Marius Vlad wrote: > igt_atomic_prepare_plane_commit() assumes that the framebuffer is always > set-up. > > Signed-off-by: Marius Vlad <marius.c.vlad@intel.com> > --- > lib/igt_kms.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/igt_kms.c b/lib/igt_kms.c > index 82257a6..30c5b7e 100644 > --- a/lib/igt_kms.c > +++ b/lib/igt_kms.c > @@ -1585,7 +1585,7 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_output_t *output, > igt_atomic_populate_plane_req(req, plane, IGT_PLANE_FB_ID, fb_id); > } > > - if (plane->position_changed || plane->size_changed) { > + if ((plane->position_changed || plane->size_changed) && fb_id != 0) { > uint32_t src_x = IGT_FIXED(plane->fb->src_x, 0); /* src_x */ > uint32_t src_y = IGT_FIXED(plane->fb->src_y, 0); /* src_y */ > uint32_t src_w = IGT_FIXED(plane->fb->src_w, 0); /* src_w */ Someone should really move the src coordinates from the fb into the plane.
Op 04-04-16 om 17:47 schreef Marius Vlad: > igt_atomic_prepare_plane_commit() assumes that the framebuffer is always > set-up. > > Signed-off-by: Marius Vlad <marius.c.vlad@intel.com> > --- > lib/igt_kms.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/igt_kms.c b/lib/igt_kms.c > index 82257a6..30c5b7e 100644 > --- a/lib/igt_kms.c > +++ b/lib/igt_kms.c > @@ -1585,7 +1585,7 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_output_t *output, > igt_atomic_populate_plane_req(req, plane, IGT_PLANE_FB_ID, fb_id); > } > > - if (plane->position_changed || plane->size_changed) { > + if ((plane->position_changed || plane->size_changed) && fb_id != 0) { > uint32_t src_x = IGT_FIXED(plane->fb->src_x, 0); /* src_x */ > uint32_t src_y = IGT_FIXED(plane->fb->src_y, 0); /* src_y */ > uint32_t src_w = IGT_FIXED(plane->fb->src_w, 0); /* src_w */ Why is this needed? ~Maarten
On Tue, Apr 05, 2016 at 10:46:37AM +0200, Maarten Lankhorst wrote: > Op 04-04-16 om 17:47 schreef Marius Vlad: > > igt_atomic_prepare_plane_commit() assumes that the framebuffer is always > > set-up. > > > > Signed-off-by: Marius Vlad <marius.c.vlad@intel.com> > > --- > > lib/igt_kms.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/lib/igt_kms.c b/lib/igt_kms.c > > index 82257a6..30c5b7e 100644 > > --- a/lib/igt_kms.c > > +++ b/lib/igt_kms.c > > @@ -1585,7 +1585,7 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_output_t *output, > > igt_atomic_populate_plane_req(req, plane, IGT_PLANE_FB_ID, fb_id); > > } > > > > - if (plane->position_changed || plane->size_changed) { > > + if ((plane->position_changed || plane->size_changed) && fb_id != 0) { > > uint32_t src_x = IGT_FIXED(plane->fb->src_x, 0); /* src_x */ > > uint32_t src_y = IGT_FIXED(plane->fb->src_y, 0); /* src_y */ > > uint32_t src_w = IGT_FIXED(plane->fb->src_w, 0); /* src_w */ > Why is this needed? Because plane->fb is NULL when fb_id == 0. Maybe Ville had a point, not having src_* in plane->fb. > > ~Maarten
Op 05-04-16 om 12:29 schreef Marius Vlad: > On Tue, Apr 05, 2016 at 10:46:37AM +0200, Maarten Lankhorst wrote: >> Op 04-04-16 om 17:47 schreef Marius Vlad: >>> igt_atomic_prepare_plane_commit() assumes that the framebuffer is always >>> set-up. >>> >>> Signed-off-by: Marius Vlad <marius.c.vlad@intel.com> >>> --- >>> lib/igt_kms.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c >>> index 82257a6..30c5b7e 100644 >>> --- a/lib/igt_kms.c >>> +++ b/lib/igt_kms.c >>> @@ -1585,7 +1585,7 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_output_t *output, >>> igt_atomic_populate_plane_req(req, plane, IGT_PLANE_FB_ID, fb_id); >>> } >>> >>> - if (plane->position_changed || plane->size_changed) { >>> + if ((plane->position_changed || plane->size_changed) && fb_id != 0) { >>> uint32_t src_x = IGT_FIXED(plane->fb->src_x, 0); /* src_x */ >>> uint32_t src_y = IGT_FIXED(plane->fb->src_y, 0); /* src_y */ >>> uint32_t src_w = IGT_FIXED(plane->fb->src_w, 0); /* src_w */ >> Why is this needed? > Because plane->fb is NULL when fb_id == 0. Maybe Ville had a point, not having > src_* in plane->fb. > Indeed, it's a property of the plane, not the fb. You could put the same fb on multiple planes with different src parameters. :) ~Maarten
diff --git a/lib/igt_kms.c b/lib/igt_kms.c index 82257a6..30c5b7e 100644 --- a/lib/igt_kms.c +++ b/lib/igt_kms.c @@ -1585,7 +1585,7 @@ igt_atomic_prepare_plane_commit(igt_plane_t *plane, igt_output_t *output, igt_atomic_populate_plane_req(req, plane, IGT_PLANE_FB_ID, fb_id); } - if (plane->position_changed || plane->size_changed) { + if ((plane->position_changed || plane->size_changed) && fb_id != 0) { uint32_t src_x = IGT_FIXED(plane->fb->src_x, 0); /* src_x */ uint32_t src_y = IGT_FIXED(plane->fb->src_y, 0); /* src_y */ uint32_t src_w = IGT_FIXED(plane->fb->src_w, 0); /* src_w */
igt_atomic_prepare_plane_commit() assumes that the framebuffer is always set-up. Signed-off-by: Marius Vlad <marius.c.vlad@intel.com> --- lib/igt_kms.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)