On Thu, Apr 16, 2020 at 03:51:36PM +0200, Yussuf Khalil wrote: > On Tue, 2020-04-14 at 14:41 +0200, Daniel Vetter wrote: > > On Mon, Apr 13, 2020 at 11:40:22PM +0200, Yussuf Khalil wrote: > > > Add a new flag to mark modes that are considered a CE mode > > > according to the > > > CEA-861 specification. Modes without this flag are implicitly > > > considered to > > > be IT modes. > > > > > > User-space applications may use this flag to determine possible > > > implications of using a CE mode (e.g., limited RGB range). > > > > > > There is no use for this flag inside the kernel, so we set it only > > > when > > > communicating a mode to user-space. > > > > > > Signed-off-by: Yussuf Khalil <dev@xxxxxxxxxx> > > > > Do we have userspace for this? > > > > If we go with the existing quant range property you don't need new > > userspace for the property itself. But this flag here is new uapi, so > > needs userspace per > > > > https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements > > > > Also since this standardizes kms uapi, we need testcases per > > > > https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#testing-requirements-for-userspace-api > > > > Cheers, Daniel > > > > > --- > > > drivers/gpu/drm/drm_modes.c | 14 ++++++++++++++ > > > include/uapi/drm/drm_mode.h | 2 ++ > > > 2 files changed, 16 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/drm_modes.c > > > b/drivers/gpu/drm/drm_modes.c > > > index d4d64518e11b..0d8a032f437d 100644 > > > --- a/drivers/gpu/drm/drm_modes.c > > > +++ b/drivers/gpu/drm/drm_modes.c > > > @@ -1973,6 +1973,14 @@ void drm_mode_convert_to_umode(struct > > > drm_mode_modeinfo *out, > > > break; > > > } > > > > > > + if (drm_match_cea_mode(in) > 1) { > > > + /* > > > + * All modes in CTA-861-G Table 1 are CE modes, except > > > 640x480p > > > + * (VIC 1). > > > + */ > > > + out->flags |= DRM_MODE_FLAG_CEA_861_CE_MODE; > > > + } > > > + > > > strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN); > > > out->name[DRM_DISPLAY_MODE_LEN-1] = 0; > > > } > > > @@ -2045,6 +2053,12 @@ int drm_mode_convert_umode(struct drm_device > > > *dev, > > > return -EINVAL; > > > } > > > > > > + /* > > > + * The CEA-861 CE mode flag is purely informational and > > > intended for > > > + * userspace only. > > > + */ > > > + out->flags &= ~DRM_MODE_FLAG_CEA_861_CE_MODE; > > > + > > > out->status = drm_mode_validate_driver(dev, out); > > > if (out->status != MODE_OK) > > > return -EINVAL; > > > diff --git a/include/uapi/drm/drm_mode.h > > > b/include/uapi/drm/drm_mode.h > > > index 735c8cfdaaa1..5e78b350b2e2 100644 > > > --- a/include/uapi/drm/drm_mode.h > > > +++ b/include/uapi/drm/drm_mode.h > > > @@ -124,6 +124,8 @@ extern "C" { > > > #define DRM_MODE_FLAG_PIC_AR_256_135 \ > > > (DRM_MODE_PICTURE_ASPECT_256_135<<19) > > > > > > +#define DRM_MODE_FLAG_CEA_861_CE_MODE (1<<23) > > > + > > > #define DRM_MODE_FLAG_ALL (DRM_MODE_FLAG_PHSYNC | \ > > > DRM_MODE_FLAG_NHSYNC | \ > > > DRM_MODE_FLAG_PVSYNC | \ > > > -- > > > 2.26.0 > > > > > Sorry, I wasn't aware DRM had these additional requirements. I do have a user- > space implementation in mutter and gnome-control-center that makes use of the > new property and this flag on my local machine. I'll try to propose the branch > upstream before sending in the next revision of this patchset. > > Do I understand it correctly that this will require test cases for both the > property itself and the new flag? I'll write a patch for IGT then. Yup. We even have some edid injection stuff so you can (for some value of "can") test this on systems without such a monitor. That would obviously be the gold standard for this, so that CI systems can make sure we don't break any of this in the driver side. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
