On 2026-09-25 12:30, Leo Li wrote: > > > On 2026-09-22 04:32, Thomas Zimmermann wrote: >> Hi >> >> Am 08.09.26 um 06:40 schrieb Mario Limonciello: >>> The upcoming per-connector LUMINANCE range property needs to accept the >>> value 0 (to turn the display off on DPMS-off) even when its advertised >>> minimum is 1. The existing check special-cased a single device-wide >>> property object, which does not work once every connector owns its own >>> LUMINANCE property. >>> >>> Add a kernel-internal is_luminance flag on struct drm_property and key >>> the value-0 exception off it instead of a pointer comparison. The flag >>> is not exposed to userspace. >> >> I don't think this is a good idea. Let userspace control display status and >> luminance independently. To my understanding, both are independent devices. >> Just because the backlight is off doesn't mean that the display is off as >> well, (right ?) >> >> Best regards >> Thomas > > Yeah, backlight off and DPMS off are quite different. The CRTC is still > active if backlight off, but that's not the case for DPMS. > > I agree that we should detangle LUMINANCE from DPMS completely: Leave > LUMINANCE alone when DPMS_OFF. If LUMINANCE is changed during DPMS_OFF, save > the state in sw. Then on DPMS_ON, restore it to panel. Sysfs seems to have > the same behavior today (I just tried it on my fw13 laptop). > > It would be good to remove references to DPMS on the LUMINANCE property in > patch 04/14 as well. >
Yeah, this property and DPMS need to be independent. They're two completely different concepts. Harry > Thanks, > Leo > >> >>> >>> Signed-off-by: Mario Limonciello (AMD) <[email protected]> >>> --- >>> drivers/gpu/drm/drm_property.c | 6 ++++++ >>> include/drm/drm_property.h | 10 ++++++++++ >>> 2 files changed, 16 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/drm_property.c b/drivers/gpu/drm/drm_property.c >>> index f38f2c5437e68..adccee10cfde4 100644 >>> --- a/drivers/gpu/drm/drm_property.c >>> +++ b/drivers/gpu/drm/drm_property.c >>> @@ -952,6 +952,12 @@ bool drm_property_change_valid_get(struct drm_property >>> *property, >>> *ref = NULL; >>> if (drm_property_type_is(property, DRM_MODE_PROP_RANGE)) { >>> + /* >>> + * Special case for the luminance property: allow 0 to turn the >>> + * display off even when the normal range starts at 1. >>> + */ >>> + if (property->is_luminance && value == 0 && property->values[1] > >>> 0) >>> + return true; >>> if (value < property->values[0] || value > property->values[1]) >>> return false; >>> return true; >>> diff --git a/include/drm/drm_property.h b/include/drm/drm_property.h >>> index aa49b5a42bb56..8bb568a4cd485 100644 >>> --- a/include/drm/drm_property.h >>> +++ b/include/drm/drm_property.h >>> @@ -188,6 +188,16 @@ struct drm_property { >>> */ >>> struct drm_device *dev; >>> + /** >>> + * @is_luminance: >>> + * >>> + * True for the per-connector LUMINANCE range property. Such a property >>> + * additionally accepts the value 0 (to turn the display off) even when >>> + * its minimum is 1. This is a kernel-internal flag and is not exposed >>> + * to userspace. >>> + */ >>> + bool is_luminance; >>> + >>> /** >>> * @enum_list: >>> * >> >
