On Mon, 09 Nov 2015, Paulo Zanoni <przanoni@xxxxxxxxx> wrote: > 2015-11-09 8:17 GMT-02:00 Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>: >> On Mon, 09 Nov 2015, "Shih-Yuan Lee (FourDollars)" <sylee@xxxxxxxxxxxxx> wrote: >>> The PWM brightness level of Dell XPS 13 (2015) is from 10 to 937 however >>> the sysfs brightness level always starts from 0 so it is better to use >>> 927 as the sysfs maximum brightness level and it becomes easier to map >>> from the PWM brightness level to the sysfs brightness level. >> >> We've been thinking we should provide a fixed range to userspace >> instead. Say, 0-100. > > While not clearly stated, this reply and the further ones sound like a > rejection to his patch. I wanted to understand the motivation rather than bluntly rejecting. > I'm not really an expert on backlight, but his idea sounds simple and > an overall improvement to the codebase. I don't think it's fair to > block a simple one-line improvement based on the fact that we have an > idea (that may or may not be implemented) for a possibly better > implementation. Why not apply his patch (in case we conclude it > doesn't have any bugs), and then go for the 0-100 idea once someone > actually decides to implement it? The implementation is diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index a24df35e11e7..d5d86601d411 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -1211,7 +1211,7 @@ static int intel_backlight_device_register(struct intel_connector *connector) * Note: Everything should work even if the backlight device max * presented to the userspace is arbitrarily chosen. */ - props.max_brightness = panel->backlight.max; + props.max_brightness = 100; props.brightness = scale_hw_to_user(connector, panel->backlight.level, props.max_brightness); and "just" testing is required. > Besides, isn't there the possibility of having a panel that has a > 0-9999 range where the change from 0 to 9990 is negligible, and only > 9991-9999 matters, so when we scale to 0-100 it will become basically > a on/off switch? We all learned to never underestimate panel hardware. That's a somewhat more valid argument. The answer to that should be that we map the userspace range to the hardware range according to a curve rather than a linear mapping. See Ville's reply. BR, Jani. > >> >> BR, >> Jani. >> >> >> >> >>> >>> Signed-off-by: Shih-Yuan Lee (FourDollars) <sylee@xxxxxxxxxxxxx> >>> --- >>> drivers/gpu/drm/i915/intel_panel.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c >>> index a24df35..697fd4d 100644 >>> --- a/drivers/gpu/drm/i915/intel_panel.c >>> +++ b/drivers/gpu/drm/i915/intel_panel.c >>> @@ -1211,7 +1211,7 @@ static int intel_backlight_device_register(struct intel_connector *connector) >>> * Note: Everything should work even if the backlight device max >>> * presented to the userspace is arbitrarily chosen. >>> */ >>> - props.max_brightness = panel->backlight.max; >>> + props.max_brightness = panel->backlight.max - panel->backlight.min; >>> props.brightness = scale_hw_to_user(connector, >>> panel->backlight.level, >>> props.max_brightness); >> >> -- >> Jani Nikula, Intel Open Source Technology Center >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > Paulo Zanoni -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
