Re: [PATCH 1/5] drm/modes: Indicate CEA-861 CE modes to user-space
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
> >
> > 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/5] drm/modes: Indicate CEA-861 CE modes to user-space
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
>
> 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.
Regards
Yussuf
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/5] drm/modes: Indicate CEA-861 CE modes to user-space
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
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
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/5] drm/modes: Indicate CEA-861 CE modes to user-space
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
---
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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
