| Message ID | 20200910092425.1016976-2-stefan@agner.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,1/3] drm: use flags instead of boolean in plane check | expand |
On Thu, Sep 10, 2020 at 11:24:24AM +0200, Stefan Agner wrote: > Add flag which checks that the framebuffer size matches the plane size > exactly. This is useful for display controller which can't handle > framebuffers other than the plane/CRTC size. > > Signed-off-by: Stefan Agner <stefan@agner.ch> > --- > drivers/gpu/drm/drm_atomic_helper.c | 7 +++++++ > drivers/gpu/drm/selftests/test-drm_plane_helper.c | 9 +++++++++ > include/drm/drm_atomic_helper.h | 1 + > 3 files changed, 17 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index 755572a37f3f..8bc7f8c2e566 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -802,6 +802,7 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state, > int hscale, vscale; > bool can_position = flags & DRM_PLANE_CAN_POSITION; > bool can_update_disabled = flags & DRM_PLANE_CAN_UPDATE_DISABLED; > + bool require_matching_fb = flags & DRM_PLANE_REQUIRE_MATCHING_FB; > > WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc); > > @@ -860,6 +861,12 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state, > return -EINVAL; > } > > + if (require_matching_fb && (drm_rect_width(src) != fb->width || > + drm_rect_height(src) != fb->height)) { > + DRM_DEBUG_KMS("Framebuffer size must match plane size.\n"); > + return -EINVAL; > + } I think you also want to check that the x,y src are at (0, 0). Plus needs kerneldoc. But aside from the details, I like. > + > return 0; > } > EXPORT_SYMBOL(drm_atomic_helper_check_plane_state); > diff --git a/drivers/gpu/drm/selftests/test-drm_plane_helper.c b/drivers/gpu/drm/selftests/test-drm_plane_helper.c > index 01e95b2d572f..2c81bbef830e 100644 > --- a/drivers/gpu/drm/selftests/test-drm_plane_helper.c > +++ b/drivers/gpu/drm/selftests/test-drm_plane_helper.c > @@ -139,6 +139,15 @@ int igt_check_plane_state(void *ignored) > FAIL_ON(!check_src_eq(&plane_state, 0, 0, 1023 << 16, 767 << 16)); > FAIL_ON(!check_crtc_eq(&plane_state, 0, 0, 1023, 767)); > > + /* Check whether requiring same size framebuffer works correctly. */ > + set_src(&plane_state, 0, 0, 1024 << 16, 768 << 16); > + set_crtc(&plane_state, 0, 0, 1024, 768); > + ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state, > + DRM_PLANE_HELPER_NO_SCALING, > + DRM_PLANE_HELPER_NO_SCALING, > + DRM_PLANE_REQUIRE_MATCHING_FB); > + FAIL(!ret, "Should not be able to use different size framebuffer with REQUIRE_MATCHING_FB\n"); I think also needs a negative test with src_x/y != 0, plus maybe one that succeeds. Cheers, Daniel > + > /* Simple scaling tests. */ > set_src(&plane_state, 0, 0, 512 << 16, 384 << 16); > set_crtc(&plane_state, 0, 0, 1024, 768); > diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h > index bb9957b4f91b..244b730e84d3 100644 > --- a/include/drm/drm_atomic_helper.h > +++ b/include/drm/drm_atomic_helper.h > @@ -43,6 +43,7 @@ int drm_atomic_helper_check_modeset(struct drm_device *dev, > > #define DRM_PLANE_CAN_POSITION BIT(0) > #define DRM_PLANE_CAN_UPDATE_DISABLED BIT(1) > +#define DRM_PLANE_REQUIRE_MATCHING_FB BIT(2) > > int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state, > const struct drm_crtc_state *crtc_state, > -- > 2.28.0 >
On Thu, Sep 10, 2020 at 11:24:24AM +0200, Stefan Agner wrote: > Add flag which checks that the framebuffer size matches the plane size > exactly. This is useful for display controller which can't handle > framebuffers other than the plane/CRTC size. > > Signed-off-by: Stefan Agner <stefan@agner.ch> > --- > drivers/gpu/drm/drm_atomic_helper.c | 7 +++++++ > drivers/gpu/drm/selftests/test-drm_plane_helper.c | 9 +++++++++ > include/drm/drm_atomic_helper.h | 1 + > 3 files changed, 17 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index 755572a37f3f..8bc7f8c2e566 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -802,6 +802,7 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state, > int hscale, vscale; > bool can_position = flags & DRM_PLANE_CAN_POSITION; > bool can_update_disabled = flags & DRM_PLANE_CAN_UPDATE_DISABLED; > + bool require_matching_fb = flags & DRM_PLANE_REQUIRE_MATCHING_FB; > > WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc); > > @@ -860,6 +861,12 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state, > return -EINVAL; > } > > + if (require_matching_fb && (drm_rect_width(src) != fb->width || > + drm_rect_height(src) != fb->height)) { src rect is .16 fixed point vs. fb dimensions are integers Still not a fan of these swiss army knife functions but meh. > + DRM_DEBUG_KMS("Framebuffer size must match plane size.\n"); > + return -EINVAL; > + } > + > return 0; > } > EXPORT_SYMBOL(drm_atomic_helper_check_plane_state); > diff --git a/drivers/gpu/drm/selftests/test-drm_plane_helper.c b/drivers/gpu/drm/selftests/test-drm_plane_helper.c > index 01e95b2d572f..2c81bbef830e 100644 > --- a/drivers/gpu/drm/selftests/test-drm_plane_helper.c > +++ b/drivers/gpu/drm/selftests/test-drm_plane_helper.c > @@ -139,6 +139,15 @@ int igt_check_plane_state(void *ignored) > FAIL_ON(!check_src_eq(&plane_state, 0, 0, 1023 << 16, 767 << 16)); > FAIL_ON(!check_crtc_eq(&plane_state, 0, 0, 1023, 767)); > > + /* Check whether requiring same size framebuffer works correctly. */ > + set_src(&plane_state, 0, 0, 1024 << 16, 768 << 16); > + set_crtc(&plane_state, 0, 0, 1024, 768); > + ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state, > + DRM_PLANE_HELPER_NO_SCALING, > + DRM_PLANE_HELPER_NO_SCALING, > + DRM_PLANE_REQUIRE_MATCHING_FB); > + FAIL(!ret, "Should not be able to use different size framebuffer with REQUIRE_MATCHING_FB\n"); > + > /* Simple scaling tests. */ > set_src(&plane_state, 0, 0, 512 << 16, 384 << 16); > set_crtc(&plane_state, 0, 0, 1024, 768); > diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h > index bb9957b4f91b..244b730e84d3 100644 > --- a/include/drm/drm_atomic_helper.h > +++ b/include/drm/drm_atomic_helper.h > @@ -43,6 +43,7 @@ int drm_atomic_helper_check_modeset(struct drm_device *dev, > > #define DRM_PLANE_CAN_POSITION BIT(0) > #define DRM_PLANE_CAN_UPDATE_DISABLED BIT(1) > +#define DRM_PLANE_REQUIRE_MATCHING_FB BIT(2) > > int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state, > const struct drm_crtc_state *crtc_state, > -- > 2.28.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On 2020-09-11 10:52, Daniel Vetter wrote: > On Thu, Sep 10, 2020 at 11:24:24AM +0200, Stefan Agner wrote: >> Add flag which checks that the framebuffer size matches the plane size >> exactly. This is useful for display controller which can't handle >> framebuffers other than the plane/CRTC size. >> >> Signed-off-by: Stefan Agner <stefan@agner.ch> >> --- >> drivers/gpu/drm/drm_atomic_helper.c | 7 +++++++ >> drivers/gpu/drm/selftests/test-drm_plane_helper.c | 9 +++++++++ >> include/drm/drm_atomic_helper.h | 1 + >> 3 files changed, 17 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c >> index 755572a37f3f..8bc7f8c2e566 100644 >> --- a/drivers/gpu/drm/drm_atomic_helper.c >> +++ b/drivers/gpu/drm/drm_atomic_helper.c >> @@ -802,6 +802,7 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state, >> int hscale, vscale; >> bool can_position = flags & DRM_PLANE_CAN_POSITION; >> bool can_update_disabled = flags & DRM_PLANE_CAN_UPDATE_DISABLED; >> + bool require_matching_fb = flags & DRM_PLANE_REQUIRE_MATCHING_FB; >> >> WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc); >> >> @@ -860,6 +861,12 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state, >> return -EINVAL; >> } >> >> + if (require_matching_fb && (drm_rect_width(src) != fb->width || >> + drm_rect_height(src) != fb->height)) { >> + DRM_DEBUG_KMS("Framebuffer size must match plane size.\n"); >> + return -EINVAL; >> + } > > I think you also want to check that the x,y src are at (0, 0). Hm, I was thinking that is handled by CAN_POSITION, but that really is all about src vs. dst in plane coordinates. I guess at that point its almost nicer to create a drm_rect for fb and use drm_rect_equals. -- Stefan > > Plus needs kerneldoc. > > But aside from the details, I like. > >> + >> return 0; >> } >> EXPORT_SYMBOL(drm_atomic_helper_check_plane_state); >> diff --git a/drivers/gpu/drm/selftests/test-drm_plane_helper.c b/drivers/gpu/drm/selftests/test-drm_plane_helper.c >> index 01e95b2d572f..2c81bbef830e 100644 >> --- a/drivers/gpu/drm/selftests/test-drm_plane_helper.c >> +++ b/drivers/gpu/drm/selftests/test-drm_plane_helper.c >> @@ -139,6 +139,15 @@ int igt_check_plane_state(void *ignored) >> FAIL_ON(!check_src_eq(&plane_state, 0, 0, 1023 << 16, 767 << 16)); >> FAIL_ON(!check_crtc_eq(&plane_state, 0, 0, 1023, 767)); >> >> + /* Check whether requiring same size framebuffer works correctly. */ >> + set_src(&plane_state, 0, 0, 1024 << 16, 768 << 16); >> + set_crtc(&plane_state, 0, 0, 1024, 768); >> + ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state, >> + DRM_PLANE_HELPER_NO_SCALING, >> + DRM_PLANE_HELPER_NO_SCALING, >> + DRM_PLANE_REQUIRE_MATCHING_FB); >> + FAIL(!ret, "Should not be able to use different size framebuffer with REQUIRE_MATCHING_FB\n"); > > I think also needs a negative test with src_x/y != 0, plus maybe one that > succeeds. > > Cheers, Daniel >> + >> /* Simple scaling tests. */ >> set_src(&plane_state, 0, 0, 512 << 16, 384 << 16); >> set_crtc(&plane_state, 0, 0, 1024, 768); >> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h >> index bb9957b4f91b..244b730e84d3 100644 >> --- a/include/drm/drm_atomic_helper.h >> +++ b/include/drm/drm_atomic_helper.h >> @@ -43,6 +43,7 @@ int drm_atomic_helper_check_modeset(struct drm_device *dev, >> >> #define DRM_PLANE_CAN_POSITION BIT(0) >> #define DRM_PLANE_CAN_UPDATE_DISABLED BIT(1) >> +#define DRM_PLANE_REQUIRE_MATCHING_FB BIT(2) >> >> int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state, >> const struct drm_crtc_state *crtc_state, >> -- >> 2.28.0 >>
On 2020-09-11 13:59, Ville Syrjälä wrote: > On Thu, Sep 10, 2020 at 11:24:24AM +0200, Stefan Agner wrote: >> Add flag which checks that the framebuffer size matches the plane size >> exactly. This is useful for display controller which can't handle >> framebuffers other than the plane/CRTC size. >> >> Signed-off-by: Stefan Agner <stefan@agner.ch> >> --- >> drivers/gpu/drm/drm_atomic_helper.c | 7 +++++++ >> drivers/gpu/drm/selftests/test-drm_plane_helper.c | 9 +++++++++ >> include/drm/drm_atomic_helper.h | 1 + >> 3 files changed, 17 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c >> index 755572a37f3f..8bc7f8c2e566 100644 >> --- a/drivers/gpu/drm/drm_atomic_helper.c >> +++ b/drivers/gpu/drm/drm_atomic_helper.c >> @@ -802,6 +802,7 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state, >> int hscale, vscale; >> bool can_position = flags & DRM_PLANE_CAN_POSITION; >> bool can_update_disabled = flags & DRM_PLANE_CAN_UPDATE_DISABLED; >> + bool require_matching_fb = flags & DRM_PLANE_REQUIRE_MATCHING_FB; >> >> WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc); >> >> @@ -860,6 +861,12 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state, >> return -EINVAL; >> } >> >> + if (require_matching_fb && (drm_rect_width(src) != fb->width || >> + drm_rect_height(src) != fb->height)) { > > src rect is .16 fixed point vs. fb dimensions are integers Good catch, will fix. Hm, I wonder why this did not trip my test. -- Stefan > > Still not a fan of these swiss army knife functions but meh. > >> + DRM_DEBUG_KMS("Framebuffer size must match plane size.\n"); >> + return -EINVAL; >> + } >> + >> return 0; >> } >> EXPORT_SYMBOL(drm_atomic_helper_check_plane_state); >> diff --git a/drivers/gpu/drm/selftests/test-drm_plane_helper.c b/drivers/gpu/drm/selftests/test-drm_plane_helper.c >> index 01e95b2d572f..2c81bbef830e 100644 >> --- a/drivers/gpu/drm/selftests/test-drm_plane_helper.c >> +++ b/drivers/gpu/drm/selftests/test-drm_plane_helper.c >> @@ -139,6 +139,15 @@ int igt_check_plane_state(void *ignored) >> FAIL_ON(!check_src_eq(&plane_state, 0, 0, 1023 << 16, 767 << 16)); >> FAIL_ON(!check_crtc_eq(&plane_state, 0, 0, 1023, 767)); >> >> + /* Check whether requiring same size framebuffer works correctly. */ >> + set_src(&plane_state, 0, 0, 1024 << 16, 768 << 16); >> + set_crtc(&plane_state, 0, 0, 1024, 768); >> + ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state, >> + DRM_PLANE_HELPER_NO_SCALING, >> + DRM_PLANE_HELPER_NO_SCALING, >> + DRM_PLANE_REQUIRE_MATCHING_FB); >> + FAIL(!ret, "Should not be able to use different size framebuffer with REQUIRE_MATCHING_FB\n"); >> + >> /* Simple scaling tests. */ >> set_src(&plane_state, 0, 0, 512 << 16, 384 << 16); >> set_crtc(&plane_state, 0, 0, 1024, 768); >> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h >> index bb9957b4f91b..244b730e84d3 100644 >> --- a/include/drm/drm_atomic_helper.h >> +++ b/include/drm/drm_atomic_helper.h >> @@ -43,6 +43,7 @@ int drm_atomic_helper_check_modeset(struct drm_device *dev, >> >> #define DRM_PLANE_CAN_POSITION BIT(0) >> #define DRM_PLANE_CAN_UPDATE_DISABLED BIT(1) >> +#define DRM_PLANE_REQUIRE_MATCHING_FB BIT(2) >> >> int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state, >> const struct drm_crtc_state *crtc_state, >> -- >> 2.28.0 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 755572a37f3f..8bc7f8c2e566 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -802,6 +802,7 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state, int hscale, vscale; bool can_position = flags & DRM_PLANE_CAN_POSITION; bool can_update_disabled = flags & DRM_PLANE_CAN_UPDATE_DISABLED; + bool require_matching_fb = flags & DRM_PLANE_REQUIRE_MATCHING_FB; WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state->crtc); @@ -860,6 +861,12 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state, return -EINVAL; } + if (require_matching_fb && (drm_rect_width(src) != fb->width || + drm_rect_height(src) != fb->height)) { + DRM_DEBUG_KMS("Framebuffer size must match plane size.\n"); + return -EINVAL; + } + return 0; } EXPORT_SYMBOL(drm_atomic_helper_check_plane_state); diff --git a/drivers/gpu/drm/selftests/test-drm_plane_helper.c b/drivers/gpu/drm/selftests/test-drm_plane_helper.c index 01e95b2d572f..2c81bbef830e 100644 --- a/drivers/gpu/drm/selftests/test-drm_plane_helper.c +++ b/drivers/gpu/drm/selftests/test-drm_plane_helper.c @@ -139,6 +139,15 @@ int igt_check_plane_state(void *ignored) FAIL_ON(!check_src_eq(&plane_state, 0, 0, 1023 << 16, 767 << 16)); FAIL_ON(!check_crtc_eq(&plane_state, 0, 0, 1023, 767)); + /* Check whether requiring same size framebuffer works correctly. */ + set_src(&plane_state, 0, 0, 1024 << 16, 768 << 16); + set_crtc(&plane_state, 0, 0, 1024, 768); + ret = drm_atomic_helper_check_plane_state(&plane_state, &crtc_state, + DRM_PLANE_HELPER_NO_SCALING, + DRM_PLANE_HELPER_NO_SCALING, + DRM_PLANE_REQUIRE_MATCHING_FB); + FAIL(!ret, "Should not be able to use different size framebuffer with REQUIRE_MATCHING_FB\n"); + /* Simple scaling tests. */ set_src(&plane_state, 0, 0, 512 << 16, 384 << 16); set_crtc(&plane_state, 0, 0, 1024, 768); diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h index bb9957b4f91b..244b730e84d3 100644 --- a/include/drm/drm_atomic_helper.h +++ b/include/drm/drm_atomic_helper.h @@ -43,6 +43,7 @@ int drm_atomic_helper_check_modeset(struct drm_device *dev, #define DRM_PLANE_CAN_POSITION BIT(0) #define DRM_PLANE_CAN_UPDATE_DISABLED BIT(1) +#define DRM_PLANE_REQUIRE_MATCHING_FB BIT(2) int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state, const struct drm_crtc_state *crtc_state,
Add flag which checks that the framebuffer size matches the plane size exactly. This is useful for display controller which can't handle framebuffers other than the plane/CRTC size. Signed-off-by: Stefan Agner <stefan@agner.ch> --- drivers/gpu/drm/drm_atomic_helper.c | 7 +++++++ drivers/gpu/drm/selftests/test-drm_plane_helper.c | 9 +++++++++ include/drm/drm_atomic_helper.h | 1 + 3 files changed, 17 insertions(+)