| Message ID | 1544789421-5265-2-git-send-email-c.manszewski@samsung.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [1/6] include/drm: color_mgmt: Add enum labels | expand |
Hi, 18. 12. 14. 오후 9:10에 Christoph Manszewski 이(가) 쓴 글: > Range setting makes sense for YCbCr and RGB buffers. Current > drm_color_range enum labels suggest use with YCbCr buffers. > Create enum labels without colorspace specification. > > Signed-off-by: Christoph Manszewski <c.manszewski@samsung.com> > --- > include/drm/drm_color_mgmt.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h > index 90ef9996d9a4..52f6d5221a0d 100644 > --- a/include/drm/drm_color_mgmt.h > +++ b/include/drm/drm_color_mgmt.h > @@ -62,6 +62,8 @@ enum drm_color_range { > DRM_COLOR_YCBCR_LIMITED_RANGE, > DRM_COLOR_YCBCR_FULL_RANGE, > DRM_COLOR_RANGE_MAX, > + DRM_COLOR_LIMITED_RANGE = DRM_COLOR_YCBCR_LIMITED_RANGE, > + DRM_COLOR_FULL_RANGE = DRM_COLOR_YCBCR_FULL_RANGE, I don't see why above two types are needed. Is there any case that the range checking behavior should be different according to the color space? And even if there is that case, I think you have to put DRM_COLOR_RANG_MAX under DRM_COLOR_FULL_RANGE. Thanks, Inki Dae > }; > > int drm_plane_create_color_properties(struct drm_plane *plane, >
On Fri, Dec 14, 2018 at 01:10:16PM +0100, Christoph Manszewski wrote: > Range setting makes sense for YCbCr and RGB buffers. Current > drm_color_range enum labels suggest use with YCbCr buffers. > Create enum labels without colorspace specification. > > Signed-off-by: Christoph Manszewski <c.manszewski@samsung.com> > --- > include/drm/drm_color_mgmt.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h > index 90ef9996d9a4..52f6d5221a0d 100644 > --- a/include/drm/drm_color_mgmt.h > +++ b/include/drm/drm_color_mgmt.h > @@ -62,6 +62,8 @@ enum drm_color_range { > DRM_COLOR_YCBCR_LIMITED_RANGE, > DRM_COLOR_YCBCR_FULL_RANGE, > DRM_COLOR_RANGE_MAX, > + DRM_COLOR_LIMITED_RANGE = DRM_COLOR_YCBCR_LIMITED_RANGE, > + DRM_COLOR_FULL_RANGE = DRM_COLOR_YCBCR_FULL_RANGE, These enum values don't mean anything really. The strings are what matter for the uapi. The default for YCbCr needs to be limited range, the default for RGB needs to be full range. So I think this would really require a separate prop for each. But is there an actual usecase for this stuff? > }; > > int drm_plane_create_color_properties(struct drm_plane *plane, > -- > 2.7.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
+CC: Hans On 17.01.2019 20:47, Ville Syrjälä wrote: > On Fri, Dec 14, 2018 at 01:10:16PM +0100, Christoph Manszewski wrote: >> Range setting makes sense for YCbCr and RGB buffers. Current >> drm_color_range enum labels suggest use with YCbCr buffers. >> Create enum labels without colorspace specification. >> >> Signed-off-by: Christoph Manszewski <c.manszewski@samsung.com> >> --- >> include/drm/drm_color_mgmt.h | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h >> index 90ef9996d9a4..52f6d5221a0d 100644 >> --- a/include/drm/drm_color_mgmt.h >> +++ b/include/drm/drm_color_mgmt.h >> @@ -62,6 +62,8 @@ enum drm_color_range { >> DRM_COLOR_YCBCR_LIMITED_RANGE, >> DRM_COLOR_YCBCR_FULL_RANGE, >> DRM_COLOR_RANGE_MAX, >> + DRM_COLOR_LIMITED_RANGE = DRM_COLOR_YCBCR_LIMITED_RANGE, >> + DRM_COLOR_FULL_RANGE = DRM_COLOR_YCBCR_FULL_RANGE, > These enum values don't mean anything really. The strings are what > matter for the uapi. > > The default for YCbCr needs to be limited range, the default for RGB > needs to be full range. So I think this would really require a separate > prop for each. > > But is there an actual usecase for this stuff? Yes, for example HDMI path (Display Controller and HDMI encoder) in Exynos5433 works on RGB buffers and do not perform any conversion if the output is also RGB, on the other side CEA861 specs says that in case of CEA modes output RGB should be in limited range, in case of non CEA modes it should be full range. The idea was to allow userspace to check (atomic_check) which ranges are acceptable in given mode and to set desired one if possible. Any better solution is welcome? Regards Andrzej > >> }; >> >> int drm_plane_create_color_properties(struct drm_plane *plane, >> -- >> 2.7.4 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Fri, Jan 18, 2019 at 03:34:18PM +0100, Andrzej Hajda wrote: > +CC: Hans > > On 17.01.2019 20:47, Ville Syrjälä wrote: > > On Fri, Dec 14, 2018 at 01:10:16PM +0100, Christoph Manszewski wrote: > >> Range setting makes sense for YCbCr and RGB buffers. Current > >> drm_color_range enum labels suggest use with YCbCr buffers. > >> Create enum labels without colorspace specification. > >> > >> Signed-off-by: Christoph Manszewski <c.manszewski@samsung.com> > >> --- > >> include/drm/drm_color_mgmt.h | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h > >> index 90ef9996d9a4..52f6d5221a0d 100644 > >> --- a/include/drm/drm_color_mgmt.h > >> +++ b/include/drm/drm_color_mgmt.h > >> @@ -62,6 +62,8 @@ enum drm_color_range { > >> DRM_COLOR_YCBCR_LIMITED_RANGE, > >> DRM_COLOR_YCBCR_FULL_RANGE, > >> DRM_COLOR_RANGE_MAX, > >> + DRM_COLOR_LIMITED_RANGE = DRM_COLOR_YCBCR_LIMITED_RANGE, > >> + DRM_COLOR_FULL_RANGE = DRM_COLOR_YCBCR_FULL_RANGE, > > These enum values don't mean anything really. The strings are what > > matter for the uapi. > > > > The default for YCbCr needs to be limited range, the default for RGB > > needs to be full range. So I think this would really require a separate > > prop for each. > > > > But is there an actual usecase for this stuff? > > > Yes, for example HDMI path (Display Controller and HDMI encoder) in > Exynos5433 works on RGB buffers and do not perform any conversion if the > output is also RGB, on the other side CEA861 specs says that in case of > CEA modes output RGB should be in limited range, in case of non CEA > modes it should be full range. > > The idea was to allow userspace to check (atomic_check) which ranges are > acceptable in given mode and to set desired one if possible. > > Any better solution is welcome? i915 has the "Broadcast RGB" property for the CEA-861 stuff. You don't have a way to implement that?
diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h index 90ef9996d9a4..52f6d5221a0d 100644 --- a/include/drm/drm_color_mgmt.h +++ b/include/drm/drm_color_mgmt.h @@ -62,6 +62,8 @@ enum drm_color_range { DRM_COLOR_YCBCR_LIMITED_RANGE, DRM_COLOR_YCBCR_FULL_RANGE, DRM_COLOR_RANGE_MAX, + DRM_COLOR_LIMITED_RANGE = DRM_COLOR_YCBCR_LIMITED_RANGE, + DRM_COLOR_FULL_RANGE = DRM_COLOR_YCBCR_FULL_RANGE, }; int drm_plane_create_color_properties(struct drm_plane *plane,
Range setting makes sense for YCbCr and RGB buffers. Current drm_color_range enum labels suggest use with YCbCr buffers. Create enum labels without colorspace specification. Signed-off-by: Christoph Manszewski <c.manszewski@samsung.com> --- include/drm/drm_color_mgmt.h | 2 ++ 1 file changed, 2 insertions(+)