| Message ID | 1415737838-9640-1-git-send-email-ullysses.a.eoff@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform: baseline_drm_intel_nightly_pass_rate->patch_applied_pass_rate
BYT: pass/total=247/348->276/348
PNV: pass/total=326/328->324/328
ILK: pass/total=329/330->327/330
IVB: pass/total=544/546->544/546
SNB: pass/total=380/380->379/380
HSW: pass/total=586/591->589/591
BDW: pass/total=435/435->433/435
-------------------------------------Detailed-------------------------------------
test_platform: test_suite, test_case, result_with_drm_intel_nightly(count, machine_id...)...->result_with_patch_applied(count, machine_id)...
BYT: Intel_gpu_tools, igt_drv_hangman_error-state-basic, BLACKLIST(1, M31)PASS(24, M31M36M29M38) -> PASS(1, M38)
BYT: Intel_gpu_tools, igt_drv_hangman_error-state-capture-blt, BLACKLIST(1, M31)PASS(24, M31M36M29M38) -> PASS(1, M38)
BYT: Intel_gpu_tools, igt_drv_hangman_error-state-capture-bsd, BLACKLIST(1, M31)PASS(24, M31M36M29M38) -> PASS(1, M38)
BYT: Intel_gpu_tools, igt_drv_hangman_error-state-capture-render, BLACKLIST(1, M31)PASS(24, M31M36M29M38) -> PASS(1, M38)
BYT: Intel_gpu_tools, igt_drv_hangman_error-state-debugfs-entry, BLACKLIST(1, M31)PASS(24, M31M36M29M38) -> PASS(1, M38)
BYT: Intel_gpu_tools, igt_drv_hangman_error-state-sysfs-entry, BLACKLIST(1, M31)PASS(24, M31M36M29M38) -> PASS(1, M38)
BYT: Intel_gpu_tools, igt_gem_bad_reloc_negative-reloc-lut, NSPT(1, M38)PASS(15, M31M29M38) -> NSPT(2, M38)PASS(2, M38)
BYT: Intel_gpu_tools, igt_gem_reset_stats_ban-ctx-render, BLACKLIST(1, M31)PASS(24, M31M36M29M38) -> PASS(1, M38)
BYT: Intel_gpu_tools, igt_gem_reset_stats_ban-render, BLACKLIST(1, M31)PASS(24, M31M36M29M38) -> PASS(1, M38)
BYT: Intel_gpu_tools, igt_gem_reset_stats_close-pending-blt, BLACKLIST(1, M31)DMESG_WARN(11, M31M36M29)PASS(13, M31M36M38) -> PASS(1, M38)
BYT: Intel_gpu_tools, igt_gem_reset_stats_close-pending-bsd, BLACKLIST(1, M31)DMESG_WARN(9, M36M29M31)PASS(15, M31M29M38M36) -> PASS(1, M38)
BYT: Intel_gpu_tools, igt_gem_reset_stats_close-pending-ctx-render, BLACKLIST(1, M31)PASS(24, M31M36M29M38) -> PASS(1, M38)
BYT: Intel_gpu_tools, igt_gem_reset_stats_close-pending-fork-blt, BLACKLIST(1, M31)DMESG_WARN(11, M31M36M29)PASS(13, M31M36M29M38) -> PASS(1, M38)
BYT: Intel_gpu_tools, igt_gem_reset_stats_close-pending-fork-bsd, BLACKLIST(1, M31)DMESG_WARN(12, M31M36M29)PASS(12, M31M36M29M38) -> PASS(1, M38)
BYT: Intel_gpu_tools, igt_gem_reset_stats_close-pending-fork-render, BLACKLIST(1, M31)PASS(24, M31M36M29M38) -> PASS(1, M38)
BYT: Intel_gpu_tools, igt_gem_reset_stats_close-pending-fork-reverse-blt, BLACKLIST(1, M31)PASS(24, M31M36M29M38) -> PASS(1, M38)
BYT: Intel_gpu_tools, igt_gem_reset_stats_close-pending-fork-reverse-bsd, BLACKLIST(1, M31)PASS(24, M31M36M29M38) -> PASS(1, M38)
BYT: Intel_gpu_tools, igt_gem_reset_stats_close-pending-fork-reverse-render, BLACKLIST(1, M31)PASS(24, M31M36M29M38) -> PASS(1, M38)
BYT: Intel_gpu_tools, igt_gem_reset_stats_close-pending-render, BLACKLIST(1, M31)PASS(24, M31M36M29M38) -> PASS(1, M38)
BYT: Intel_gpu_tools, igt_gem_reset_stats_params, BLACKLIST(1, M31)PASS(24, M31M36M29M38) -> PASS(1, M38)
BYT: Intel_gpu_tools, igt_gem_reset_stats_params-ctx-render, BLACKLIST(1, M31)PASS(24, M31M36M29M38) -> PASS(1, M38)
BYT: Intel_gpu_tools, igt_gem_reset_stats_reset-count-blt, BLACKLIST(1, M31)DMESG_WARN(16, M31M36M29)PASS(8, M29M38M36) -> PASS(1, M38)
BYT: Intel_gpu_tools, igt_gem_reset_stats_reset-count-bsd, BLACKLIST(1, M31)DMESG_WARN(14, M31M36M29)PASS(10, M36M38M29) -> PASS(1, M38)
BYT: Intel_gpu_tools, igt_gem_reset_stats_reset-count-ctx-render, BLACKLIST(1, M31)PASS(24, M31M36M29M38) -> PASS(1, M38)
BYT: Intel_gpu_tools, igt_gem_reset_stats_reset-count-render, BLACKLIST(1, M31)PASS(24, M31M36M29M38) -> PASS(1, M38)
BYT: Intel_gpu_tools, igt_gem_reset_stats_reset-stats-blt, BLACKLIST(1, M31)DMESG_WARN(8, M36M29M31)PASS(16, M31M36M29M38) -> PASS(1, M38)
BYT: Intel_gpu_tools, igt_gem_reset_stats_reset-stats-bsd, BLACKLIST(1, M31)DMESG_WARN(9, M36M29M31)PASS(15, M31M36M29M38) -> PASS(1, M38)
BYT: Intel_gpu_tools, igt_gem_reset_stats_reset-stats-ctx-render, BLACKLIST(1, M31)PASS(24, M31M36M29M38) -> PASS(1, M38)
BYT: Intel_gpu_tools, igt_gem_reset_stats_reset-stats-render, BLACKLIST(1, M31)PASS(24, M31M36M29M38) -> PASS(1, M38)
BYT: Intel_gpu_tools, igt_gem_reset_stats_unrelated-ctx-render, BLACKLIST(1, M31)PASS(24, M31M36M29M38) -> PASS(1, M38)
BYT: Intel_gpu_tools, igt_drv_hangman_ring-stop-sysfs-entry, BLACKLIST(1, M31)PASS(24, M31M36M29M38) -> PASS(1, M38)
PNV: Intel_gpu_tools, igt_gem_concurrent_blit_gpu-bcs-early-read, PASS(7, M25M23) -> DMESG_WARN(1, M25)PASS(3, M25)
PNV: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, DMESG_WARN(1, M25)TIMEOUT(15, M7M25M24M23)PASS(3, M24) -> DMESG_WARN(3, M25)TIMEOUT(1, M25)
PNV: Intel_gpu_tools, igt_gem_concurrent_blit_gttX-bcs-gpu-read-after-write-interruptible, PASS(7, M25) -> DMESG_WARN(1, M25)PASS(3, M25)
ILK: Intel_gpu_tools, igt_kms_3d, PASS(4, M26) -> DMESG_WARN(1, M26)PASS(3, M26)
ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-expired-vblank, DMESG_WARN(1, M26)PASS(3, M26) -> DMESG_WARN(2, M26)PASS(2, M26)
ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-modeset-interruptible, DMESG_WARN(2, M26)PASS(26, M37M26M6) -> DMESG_WARN(1, M26)PASS(3, M26)
ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-rmfb, PASS(7, M26) -> DMESG_WARN(1, M26)PASS(3, M26)
IVB: Intel_gpu_tools, igt_kms_plane_plane-position-covered-pipe-A-plane-1, TIMEOUT(1, M34)PASS(27, M21M4M34) -> PASS(4, M21)
IVB: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, TIMEOUT(15, M34M21M4)PASS(1, M34) -> TIMEOUT(1, M21)PASS(3, M21)
SNB: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, TIMEOUT(9, M35M22)PASS(1, M35) -> TIMEOUT(1, M35)PASS(3, M35)
HSW: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, PASS(1, M40) -> TIMEOUT(1, M40)PASS(3, M40)
HSW: Intel_gpu_tools, igt_pm_rpm_legacy-planes, TIMEOUT(1, M40) -> TIMEOUT(3, M40)PASS(1, M40)
HSW: Intel_gpu_tools, igt_pm_rpm_legacy-planes-dpms, TIMEOUT(1, M40) -> TIMEOUT(3, M40)PASS(1, M40)
HSW: Intel_gpu_tools, igt_pm_rpm_universal-planes, TIMEOUT(1, M40) -> TIMEOUT(2, M40)PASS(1, M40)
HSW: Intel_gpu_tools, igt_pm_rpm_universal-planes-dpms, TIMEOUT(1, M40) -> PASS(1, M40)
BDW: Intel_gpu_tools, igt_kms_setmode_invalid-clone-single-crtc, TIMEOUT(15, M28M42M30)PASS(1, M42) -> TIMEOUT(1, M42)PASS(3, M42)
BDW: Intel_gpu_tools, igt_gem_reset_stats_ban-bsd, PASS(31, M42M30M28) -> DMESG_WARN(1, M42)PASS(3, M42)
Jani/Jesse, Does this patch look reasonable to you for merging? -- U. Artie > -----Original Message----- > From: Eoff, Ullysses A > Sent: Tuesday, November 11, 2014 12:31 PM > To: intel-gfx@lists.freedesktop.org > Cc: Eoff, Ullysses A > Subject: [PATCH] drm/i915: expose a fixed brightness range to userspace > > A userspace brightness range that is larger than the hardware > brightness range does not have a 1:1 mapping and can result > in brightness != actual_brightness for some values. > > Expose a fixed brightness range of [0..1000] to userspace so > that all values can map 1:1 into the hardware brightness > range. This would assume that the hardware range is always > greater than 1000, otherwise we're right back to the original > issue. > > This patch is based on Jani Nikula's proposed change in the > referenced ML thread, except use the range [0..1000] instead; > which was recommended by Jesse Barnes for smoother backlight > transitions. > > Reference: http://lists.freedesktop.org/archives/intel-gfx/2014-November/055221.html > Signed-off-by: U. Artie Eoff <ullysses.a.eoff@intel.com> > --- > 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 4d63839..f74f5f2 100644 > --- a/drivers/gpu/drm/i915/intel_panel.c > +++ b/drivers/gpu/drm/i915/intel_panel.c > @@ -1057,7 +1057,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 = 1000; > props.brightness = scale_hw_to_user(connector, > panel->backlight.level, > props.max_brightness); > -- > 1.9.3
On Tue, 11 Nov 2014 12:30:38 -0800 "U. Artie Eoff" <ullysses.a.eoff@intel.com> wrote: > A userspace brightness range that is larger than the hardware > brightness range does not have a 1:1 mapping and can result > in brightness != actual_brightness for some values. > > Expose a fixed brightness range of [0..1000] to userspace so > that all values can map 1:1 into the hardware brightness > range. This would assume that the hardware range is always > greater than 1000, otherwise we're right back to the original > issue. > > This patch is based on Jani Nikula's proposed change in the > referenced ML thread, except use the range [0..1000] instead; > which was recommended by Jesse Barnes for smoother backlight > transitions. > > Reference: http://lists.freedesktop.org/archives/intel-gfx/2014-November/055221.html > Signed-off-by: U. Artie Eoff <ullysses.a.eoff@intel.com> > --- > 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 4d63839..f74f5f2 100644 > --- a/drivers/gpu/drm/i915/intel_panel.c > +++ b/drivers/gpu/drm/i915/intel_panel.c > @@ -1057,7 +1057,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 = 1000; > props.brightness = scale_hw_to_user(connector, > panel->backlight.level, > props.max_brightness); Yeah looks ok to me. I guess Jani has to ack it too. Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Thanks Jesse for the ack. Unfortunately I just learned from Stéphane that there are certain devices which only support 256 levels, so this patch would do us no good at solving the real issue for such devices. Why can't we just use a dynamic 1:1 mapping as was suggested before? I would vote for that instead. ---- U. Artie > -----Original Message----- > From: Jesse Barnes [mailto:jbarnes@virtuousgeek.org] > Sent: Tuesday, November 18, 2014 9:23 AM > To: Eoff, Ullysses A > Cc: intel-gfx@lists.freedesktop.org; Jani Nikula > Subject: Re: [Intel-gfx] [PATCH] drm/i915: expose a fixed brightness range to userspace > > On Tue, 11 Nov 2014 12:30:38 -0800 > "U. Artie Eoff" <ullysses.a.eoff@intel.com> wrote: > > > A userspace brightness range that is larger than the hardware > > brightness range does not have a 1:1 mapping and can result > > in brightness != actual_brightness for some values. > > > > Expose a fixed brightness range of [0..1000] to userspace so > > that all values can map 1:1 into the hardware brightness > > range. This would assume that the hardware range is always > > greater than 1000, otherwise we're right back to the original > > issue. > > > > This patch is based on Jani Nikula's proposed change in the > > referenced ML thread, except use the range [0..1000] instead; > > which was recommended by Jesse Barnes for smoother backlight > > transitions. > > > > Reference: http://lists.freedesktop.org/archives/intel-gfx/2014-November/055221.html > > Signed-off-by: U. Artie Eoff <ullysses.a.eoff@intel.com> > > --- > > 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 4d63839..f74f5f2 100644 > > --- a/drivers/gpu/drm/i915/intel_panel.c > > +++ b/drivers/gpu/drm/i915/intel_panel.c > > @@ -1057,7 +1057,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 = 1000; > > props.brightness = scale_hw_to_user(connector, > > panel->backlight.level, > > props.max_brightness); > > Yeah looks ok to me. I guess Jani has to ack it too. > > Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > -- > Jesse Barnes, Intel Open Source Technology Center
We can still map 1000 entries back to 256, it will just mean some of them won't result in any brightness change. If that's not acceptable, I guess a dynamic 1:1 mapping is sufficient. Jesse On Tue, 18 Nov 2014 23:18:35 +0000 "Eoff, Ullysses A" <ullysses.a.eoff@intel.com> wrote: > Thanks Jesse for the ack. > > Unfortunately I just learned from Stéphane that there > are certain devices which only support 256 levels, so this > patch would do us no good at solving the real issue for > such devices. > > Why can't we just use a dynamic 1:1 mapping as was > suggested before? I would vote for that instead. > > ---- > U. Artie > > > -----Original Message----- > > From: Jesse Barnes [mailto:jbarnes@virtuousgeek.org] > > Sent: Tuesday, November 18, 2014 9:23 AM > > To: Eoff, Ullysses A > > Cc: intel-gfx@lists.freedesktop.org; Jani Nikula > > Subject: Re: [Intel-gfx] [PATCH] drm/i915: expose a fixed brightness range to userspace > > > > On Tue, 11 Nov 2014 12:30:38 -0800 > > "U. Artie Eoff" <ullysses.a.eoff@intel.com> wrote: > > > > > A userspace brightness range that is larger than the hardware > > > brightness range does not have a 1:1 mapping and can result > > > in brightness != actual_brightness for some values. > > > > > > Expose a fixed brightness range of [0..1000] to userspace so > > > that all values can map 1:1 into the hardware brightness > > > range. This would assume that the hardware range is always > > > greater than 1000, otherwise we're right back to the original > > > issue. > > > > > > This patch is based on Jani Nikula's proposed change in the > > > referenced ML thread, except use the range [0..1000] instead; > > > which was recommended by Jesse Barnes for smoother backlight > > > transitions. > > > > > > Reference: http://lists.freedesktop.org/archives/intel-gfx/2014-November/055221.html > > > Signed-off-by: U. Artie Eoff <ullysses.a.eoff@intel.com> > > > --- > > > 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 4d63839..f74f5f2 100644 > > > --- a/drivers/gpu/drm/i915/intel_panel.c > > > +++ b/drivers/gpu/drm/i915/intel_panel.c > > > @@ -1057,7 +1057,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 = 1000; > > > props.brightness = scale_hw_to_user(connector, > > > panel->backlight.level, > > > props.max_brightness); > > > > Yeah looks ok to me. I guess Jani has to ack it too. > > > > Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > > > -- > > Jesse Barnes, Intel Open Source Technology Center >
> -----Original Message----- > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Eoff, Ullysses A > Sent: Tuesday, November 18, 2014 3:19 PM > To: Jani Nikula; Jesse Barnes > Cc: stephane.marchesin@gmail.com; intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH] drm/i915: expose a fixed brightness range to userspace > > Thanks Jesse for the ack. > > Unfortunately I just learned from Stéphane that there > are certain devices which only support 256 levels, so this > patch would do us no good at solving the real issue for > such devices. > > Why can't we just use a dynamic 1:1 mapping as was > suggested before? I would vote for that instead. > > ---- > U. Artie In all fairness, I guess Jesse did elude to the fact that there would be such devices in the previous thread: <http://lists.freedesktop.org/archives/intel-gfx/2014-November/055221.html> I don't see why we shouldn't just expose the full 1:1 range to userspace. And let userspace decide what level stepping makes sense for providing smooth backlight level transitions to the end-user if necessary. --U. Artie > > > -----Original Message----- > > From: Jesse Barnes [mailto:jbarnes@virtuousgeek.org] > > Sent: Tuesday, November 18, 2014 9:23 AM > > To: Eoff, Ullysses A > > Cc: intel-gfx@lists.freedesktop.org; Jani Nikula > > Subject: Re: [Intel-gfx] [PATCH] drm/i915: expose a fixed brightness range to userspace > > > > On Tue, 11 Nov 2014 12:30:38 -0800 > > "U. Artie Eoff" <ullysses.a.eoff@intel.com> wrote: > > > > > A userspace brightness range that is larger than the hardware > > > brightness range does not have a 1:1 mapping and can result > > > in brightness != actual_brightness for some values. > > > > > > Expose a fixed brightness range of [0..1000] to userspace so > > > that all values can map 1:1 into the hardware brightness > > > range. This would assume that the hardware range is always > > > greater than 1000, otherwise we're right back to the original > > > issue. > > > > > > This patch is based on Jani Nikula's proposed change in the > > > referenced ML thread, except use the range [0..1000] instead; > > > which was recommended by Jesse Barnes for smoother backlight > > > transitions. > > > > > > Reference: http://lists.freedesktop.org/archives/intel-gfx/2014-November/055221.html > > > Signed-off-by: U. Artie Eoff <ullysses.a.eoff@intel.com> > > > --- > > > 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 4d63839..f74f5f2 100644 > > > --- a/drivers/gpu/drm/i915/intel_panel.c > > > +++ b/drivers/gpu/drm/i915/intel_panel.c > > > @@ -1057,7 +1057,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 = 1000; > > > props.brightness = scale_hw_to_user(connector, > > > panel->backlight.level, > > > props.max_brightness); > > > > Yeah looks ok to me. I guess Jani has to ack it too. > > > > Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > > > -- > > Jesse Barnes, Intel Open Source Technology Center > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Nov 18, 2014 at 3:18 PM, Eoff, Ullysses A <ullysses.a.eoff@intel.com> wrote: > Thanks Jesse for the ack. > > Unfortunately I just learned from Stéphane that there > are certain devices which only support 256 levels, so this > patch would do us no good at solving the real issue for > such devices. > > Why can't we just use a dynamic 1:1 mapping as was > suggested before? I would vote for that instead. > Right, from my (consumer's) perspective, a 1:1 mapping is simpler. But the confusing part for me is that (as far as I can see) the current mapping should be 1:1 (because the user and hw ranges are the same), even though it goes through the scale logic. Is the scale() function maybe not the identity? If it isn't, maybe we just need to make it so... Stéphane > ---- > U. Artie > >> -----Original Message----- >> From: Jesse Barnes [mailto:jbarnes@virtuousgeek.org] >> Sent: Tuesday, November 18, 2014 9:23 AM >> To: Eoff, Ullysses A >> Cc: intel-gfx@lists.freedesktop.org; Jani Nikula >> Subject: Re: [Intel-gfx] [PATCH] drm/i915: expose a fixed brightness range to userspace >> >> On Tue, 11 Nov 2014 12:30:38 -0800 >> "U. Artie Eoff" <ullysses.a.eoff@intel.com> wrote: >> >> > A userspace brightness range that is larger than the hardware >> > brightness range does not have a 1:1 mapping and can result >> > in brightness != actual_brightness for some values. >> > >> > Expose a fixed brightness range of [0..1000] to userspace so >> > that all values can map 1:1 into the hardware brightness >> > range. This would assume that the hardware range is always >> > greater than 1000, otherwise we're right back to the original >> > issue. >> > >> > This patch is based on Jani Nikula's proposed change in the >> > referenced ML thread, except use the range [0..1000] instead; >> > which was recommended by Jesse Barnes for smoother backlight >> > transitions. >> > >> > Reference: http://lists.freedesktop.org/archives/intel-gfx/2014-November/055221.html >> > Signed-off-by: U. Artie Eoff <ullysses.a.eoff@intel.com> >> > --- >> > 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 4d63839..f74f5f2 100644 >> > --- a/drivers/gpu/drm/i915/intel_panel.c >> > +++ b/drivers/gpu/drm/i915/intel_panel.c >> > @@ -1057,7 +1057,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 = 1000; >> > props.brightness = scale_hw_to_user(connector, >> > panel->backlight.level, >> > props.max_brightness); >> >> Yeah looks ok to me. I guess Jani has to ack it too. >> >> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> >> >> -- >> Jesse Barnes, Intel Open Source Technology Center
> -----Original Message----- > From: Stéphane Marchesin [mailto:stephane.marchesin@gmail.com] > Sent: Tuesday, November 18, 2014 3:53 PM > To: Eoff, Ullysses A > Cc: Jani Nikula; Jesse Barnes; intel-gfx@lists.freedesktop.org > Subject: Re: [Intel-gfx] [PATCH] drm/i915: expose a fixed brightness range to userspace > > On Tue, Nov 18, 2014 at 3:18 PM, Eoff, Ullysses A > <ullysses.a.eoff@intel.com> wrote: > > Thanks Jesse for the ack. > > > > Unfortunately I just learned from Stéphane that there > > are certain devices which only support 256 levels, so this > > patch would do us no good at solving the real issue for > > such devices. > > > > Why can't we just use a dynamic 1:1 mapping as was > > suggested before? I would vote for that instead. > > > > Right, from my (consumer's) perspective, a 1:1 mapping is simpler. But > the confusing part for me is that (as far as I can see) the current > mapping should be 1:1 (because the user and hw ranges are the same), > even though it goes through the scale logic. Is the scale() function > maybe not the identity? If it isn't, maybe we just need to make it > so... > Yes, if the user and hw ranges are the same, then there will be a 1:1 mapping, currently, and no issue. It's the other case where the hw range is smaller than the user range we end up with brightness != actual_brightness in sysfs. The scale logic rounds into discrete values of the ranges where multiple user values can scale to the same hw value in this case. Right now, user range is [0..max hw] and hw range is [min_hw..max_hw]. If min_hw > 0, then we encounter the problem. The proposal is to set the user range to [0..(hw_max - hw_min)]. --U. Artie > Stéphane > > > > ---- > > U. Artie > > > >> -----Original Message----- > >> From: Jesse Barnes [mailto:jbarnes@virtuousgeek.org] > >> Sent: Tuesday, November 18, 2014 9:23 AM > >> To: Eoff, Ullysses A > >> Cc: intel-gfx@lists.freedesktop.org; Jani Nikula > >> Subject: Re: [Intel-gfx] [PATCH] drm/i915: expose a fixed brightness range to userspace > >> > >> On Tue, 11 Nov 2014 12:30:38 -0800 > >> "U. Artie Eoff" <ullysses.a.eoff@intel.com> wrote: > >> > >> > A userspace brightness range that is larger than the hardware > >> > brightness range does not have a 1:1 mapping and can result > >> > in brightness != actual_brightness for some values. > >> > > >> > Expose a fixed brightness range of [0..1000] to userspace so > >> > that all values can map 1:1 into the hardware brightness > >> > range. This would assume that the hardware range is always > >> > greater than 1000, otherwise we're right back to the original > >> > issue. > >> > > >> > This patch is based on Jani Nikula's proposed change in the > >> > referenced ML thread, except use the range [0..1000] instead; > >> > which was recommended by Jesse Barnes for smoother backlight > >> > transitions. > >> > > >> > Reference: http://lists.freedesktop.org/archives/intel-gfx/2014-November/055221.html > >> > Signed-off-by: U. Artie Eoff <ullysses.a.eoff@intel.com> > >> > --- > >> > 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 4d63839..f74f5f2 100644 > >> > --- a/drivers/gpu/drm/i915/intel_panel.c > >> > +++ b/drivers/gpu/drm/i915/intel_panel.c > >> > @@ -1057,7 +1057,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 = 1000; > >> > props.brightness = scale_hw_to_user(connector, > >> > panel->backlight.level, > >> > props.max_brightness); > >> > >> Yeah looks ok to me. I guess Jani has to ack it too. > >> > >> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> > >> > >> -- > >> Jesse Barnes, Intel Open Source Technology Center
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index 4d63839..f74f5f2 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -1057,7 +1057,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 = 1000; props.brightness = scale_hw_to_user(connector, panel->backlight.level, props.max_brightness);
A userspace brightness range that is larger than the hardware brightness range does not have a 1:1 mapping and can result in brightness != actual_brightness for some values. Expose a fixed brightness range of [0..1000] to userspace so that all values can map 1:1 into the hardware brightness range. This would assume that the hardware range is always greater than 1000, otherwise we're right back to the original issue. This patch is based on Jani Nikula's proposed change in the referenced ML thread, except use the range [0..1000] instead; which was recommended by Jesse Barnes for smoother backlight transitions. Reference: http://lists.freedesktop.org/archives/intel-gfx/2014-November/055221.html Signed-off-by: U. Artie Eoff <ullysses.a.eoff@intel.com> --- drivers/gpu/drm/i915/intel_panel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)