All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Allow user to override PWM backlight frequency and duty cycle
@ 2018-01-16 13:40 Alex Ivanov
  2018-01-16 14:41 ` ✗ Fi.CI.BAT: failure for " Patchwork
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Alex Ivanov @ 2018-01-16 13:40 UTC (permalink / raw)
  To: intel-gfx

Since vendor may set bad defaults or user just generally may prefer her health over
display life.
1. Allow overriding of PWM frequency otherwise people use solutions like
http://devbraindom.blogspot.ru/2013/03/eliminate-led-screen-flicker-with-intel.html
which are bad because
I. frequency convertation logic varies from chip to chip, so people set totally wrong
frequences in most cases
II. writing to backlight control registers directly goes into conflict with
i915 driver doing the same
2. Allow to override minimal brightness. Even if vendor limit was totally correct,
it may be too bright to use in night, plus the default will become most likely wrong
now, since frequency was overriden

---
 drivers/gpu/drm/i915/i915_params.c |  6 ++++++
 drivers/gpu/drm/i915/i915_params.h |  2 ++
 drivers/gpu/drm/i915/intel_panel.c | 17 +++++++++++++++++
 3 files changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index b4faeb6aa2bd..ba37257846b0 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -190,3 +190,9 @@ i915_param_named(enable_dpcd_backlight, bool, 0600,
 
 i915_param_named(enable_gvt, bool, 0400,
 	"Enable support for Intel GVT-g graphics virtualization host support(default:false)");
+
+i915_param_named_unsafe(backlight_freq, uint, 0400,
+	"Override PWM backlight frequency (set in Hz, 0=ignore [default])");
+
+i915_param_named_unsafe(backlight_min_level, uint, 0400,
+	"Override PWM backlight minimum level (duty cycle, 0=ignore [default])");
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index c7292268ed43..7aba85db3da2 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -53,6 +53,8 @@
 	param(int, edp_vswing, 0) \
 	param(int, reset, 2) \
 	param(unsigned int, inject_load_failure, 0) \
+	param(unsigned int, backlight_freq, 0) \
+	param(unsigned int, backlight_min_level, 0) \
 	/* leave bools at the end to not create holes */ \
 	param(bool, alpha_support, IS_ENABLED(CONFIG_DRM_I915_ALPHA_SUPPORT)) \
 	param(bool, enable_cmd_parser, true) \
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index adc51e452e3e..f188d7c1d2de 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1479,6 +1479,12 @@ static u32 get_backlight_min_vbt(struct intel_connector *connector)
 
 	WARN_ON(panel->backlight.max == 0);
 
+	if (i915_modparams.backlight_min_level) {
+		DRM_DEBUG_KMS("Override backlight duty cycle %u\n",
+						i915_modparams.backlight_min_level);
+		return i915_modparams.backlight_min_level;
+	}
+
 	/*
 	 * XXX: If the vbt value is 255, it makes min equal to max, which leads
 	 * to problems. There are such machines out there. Either our
@@ -1795,6 +1801,7 @@ int intel_panel_setup_backlight(struct drm_connector *connector, enum pipe pipe)
 	struct intel_connector *intel_connector = to_intel_connector(connector);
 	struct intel_panel *panel = &intel_connector->panel;
 	int ret;
+	u32 pwm;
 
 	if (!dev_priv->vbt.backlight.present) {
 		if (dev_priv->quirks & QUIRK_BACKLIGHT_PRESENT) {
@@ -1812,6 +1819,16 @@ int intel_panel_setup_backlight(struct drm_connector *connector, enum pipe pipe)
 	/* set level and max in panel struct */
 	mutex_lock(&dev_priv->backlight_lock);
 	ret = panel->backlight.setup(intel_connector, pipe);
+	if (i915_modparams.backlight_freq) {
+		if (panel->backlight.hz_to_pwm &&
+				(pwm = panel->backlight.hz_to_pwm(intel_connector, i915_modparams.backlight_freq))) {
+			panel->backlight.max = pwm;
+			DRM_DEBUG_KMS("Override backlight frequency %u Hz\n",
+							i915_modparams.backlight_freq);
+		} else {
+			DRM_DEBUG_KMS("failed to override backlight frequency\n");
+		}
+	}
 	mutex_unlock(&dev_priv->backlight_lock);
 
 	if (ret) {
-- 
2.15.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* ✗ Fi.CI.BAT: failure for drm/i915: Allow user to override PWM backlight frequency and duty cycle
  2018-01-16 13:40 [PATCH] drm/i915: Allow user to override PWM backlight frequency and duty cycle Alex Ivanov
@ 2018-01-16 14:41 ` Patchwork
  2018-01-16 15:36   ` gnidorah
  2018-01-17  7:22 ` [PATCH] " Alex Ivanov
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Patchwork @ 2018-01-16 14:41 UTC (permalink / raw)
  To: Alex Ivanov; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Allow user to override PWM backlight frequency and duty cycle
URL   : https://patchwork.freedesktop.org/series/36540/
State : failure

== Summary ==

Applying: drm/i915: Allow user to override PWM backlight frequency and duty cycle
error: Failed to merge in the changes.
Using index info to reconstruct a base tree...
M	drivers/gpu/drm/i915/i915_params.c
M	drivers/gpu/drm/i915/i915_params.h
M	drivers/gpu/drm/i915/intel_panel.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/intel_panel.c
Auto-merging drivers/gpu/drm/i915/i915_params.h
Auto-merging drivers/gpu/drm/i915/i915_params.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/i915_params.c
Patch failed at 0001 drm/i915: Allow user to override PWM backlight frequency and duty cycle
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: ✗ Fi.CI.BAT: failure for drm/i915: Allow user to override PWM backlight frequency and duty cycle
  2018-01-16 14:41 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2018-01-16 15:36   ` gnidorah
  2018-01-16 19:26     ` Saarinen, Jani
  0 siblings, 1 reply; 9+ messages in thread
From: gnidorah @ 2018-01-16 15:36 UTC (permalink / raw)
  To: intel-gfx

What branch CI tests against?
I've created the patch against current torvalds/linux

16.01.2018, 17:42, "Patchwork" <patchwork@emeril.freedesktop.org>:
> == Series Details ==
>
> Series: drm/i915: Allow user to override PWM backlight frequency and duty cycle
> URL : https://patchwork.freedesktop.org/series/36540/
> State : failure
>
> == Summary ==
>
> Applying: drm/i915: Allow user to override PWM backlight frequency and duty cycle
> error: Failed to merge in the changes.
> Using index info to reconstruct a base tree...
> M drivers/gpu/drm/i915/i915_params.c
> M drivers/gpu/drm/i915/i915_params.h
> M drivers/gpu/drm/i915/intel_panel.c
> Falling back to patching base and 3-way merge...
> Auto-merging drivers/gpu/drm/i915/intel_panel.c
> Auto-merging drivers/gpu/drm/i915/i915_params.h
> Auto-merging drivers/gpu/drm/i915/i915_params.c
> CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/i915_params.c
> Patch failed at 0001 drm/i915: Allow user to override PWM backlight frequency and duty cycle
> The copy of the patch that failed is found in: .git/rebase-apply/patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: ✗ Fi.CI.BAT: failure for drm/i915: Allow user to override PWM backlight frequency and duty cycle
  2018-01-16 15:36   ` gnidorah
@ 2018-01-16 19:26     ` Saarinen, Jani
  0 siblings, 0 replies; 9+ messages in thread
From: Saarinen, Jani @ 2018-01-16 19:26 UTC (permalink / raw)
  To: gnidorah, intel-gfx

HI, 
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
> gnidorah@ya.ru
> Sent: tiistai 16. tammikuuta 2018 17.36
> To: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Allow user to override
> PWM backlight frequency and duty cycle
> 
> What branch CI tests against?
https://cgit.freedesktop.org/drm-tip

> I've created the patch against current torvalds/linux
> 
> 16.01.2018, 17:42, "Patchwork" <patchwork@emeril.freedesktop.org>:
> > == Series Details ==
> >
> > Series: drm/i915: Allow user to override PWM backlight frequency and
> > duty cycle URL : https://patchwork.freedesktop.org/series/36540/
> > State : failure
> >
> > == Summary ==
> >
> > Applying: drm/i915: Allow user to override PWM backlight frequency and
> > duty cycle
> > error: Failed to merge in the changes.
> > Using index info to reconstruct a base tree...
> > M drivers/gpu/drm/i915/i915_params.c
> > M drivers/gpu/drm/i915/i915_params.h
> > M drivers/gpu/drm/i915/intel_panel.c
> > Falling back to patching base and 3-way merge...
> > Auto-merging drivers/gpu/drm/i915/intel_panel.c
> > Auto-merging drivers/gpu/drm/i915/i915_params.h
> > Auto-merging drivers/gpu/drm/i915/i915_params.c
> > CONFLICT (content): Merge conflict in
> > drivers/gpu/drm/i915/i915_params.c
> > Patch failed at 0001 drm/i915: Allow user to override PWM backlight
> > frequency and duty cycle The copy of the patch that failed is found
> > in: .git/rebase-apply/patch When you have resolved this problem, run "git am
> --continue".
> > If you prefer to skip this patch, run "git am --skip" instead.
> > To restore the original branch and stop patching, run "git am --abort".


Jani Saarinen
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo



 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] drm/i915: Allow user to override PWM backlight frequency and duty cycle
  2018-01-16 13:40 [PATCH] drm/i915: Allow user to override PWM backlight frequency and duty cycle Alex Ivanov
  2018-01-16 14:41 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2018-01-17  7:22 ` Alex Ivanov
  2018-01-17  7:46 ` ✓ Fi.CI.BAT: success for drm/i915: Allow user to override PWM backlight frequency and duty cycle (rev2) Patchwork
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Alex Ivanov @ 2018-01-17  7:22 UTC (permalink / raw)
  To: intel-gfx

Fixed to merge against drm-tip branch

Signed-off-by: Alex Ivanov <gnidorah@ya.ru>
---
 drivers/gpu/drm/i915/i915_params.c |  6 ++++++
 drivers/gpu/drm/i915/i915_params.h |  2 ++
 drivers/gpu/drm/i915/intel_panel.c | 17 +++++++++++++++++
 3 files changed, 25 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index b5f3eb4fa8a3..2a7b0c20f1d0 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -175,6 +175,12 @@ i915_param_named(enable_dpcd_backlight, bool, 0600,
 i915_param_named(enable_gvt, bool, 0400,
 	"Enable support for Intel GVT-g graphics virtualization host support(default:false)");
 
+i915_param_named_unsafe(backlight_freq, uint, 0400,
+	"Override PWM backlight frequency (set in Hz, 0=ignore [default])");
+
+i915_param_named_unsafe(backlight_min_level, uint, 0400,
+	"Override PWM backlight minimum level (duty cycle, 0=ignore [default])");
+
 static __always_inline void _print_param(struct drm_printer *p,
 					 const char *name,
 					 const char *type,
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index c96360398072..5e2c077c6f2c 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -55,6 +55,8 @@ struct drm_printer;
 	param(int, edp_vswing, 0) \
 	param(int, reset, 2) \
 	param(unsigned int, inject_load_failure, 0) \
+	param(unsigned int, backlight_freq, 0) \
+	param(unsigned int, backlight_min_level, 0) \
 	/* leave bools at the end to not create holes */ \
 	param(bool, alpha_support, IS_ENABLED(CONFIG_DRM_I915_ALPHA_SUPPORT)) \
 	param(bool, enable_cmd_parser, true) \
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index fa6831f8c004..eb5f0665b5f9 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -1479,6 +1479,12 @@ static u32 get_backlight_min_vbt(struct intel_connector *connector)
 
 	WARN_ON(panel->backlight.max == 0);
 
+	if (i915_modparams.backlight_min_level) {
+		DRM_DEBUG_KMS("Override backlight duty cycle %u\n",
+						i915_modparams.backlight_min_level);
+		return i915_modparams.backlight_min_level;
+	}
+
 	/*
 	 * XXX: If the vbt value is 255, it makes min equal to max, which leads
 	 * to problems. There are such machines out there. Either our
@@ -1795,6 +1801,7 @@ int intel_panel_setup_backlight(struct drm_connector *connector, enum pipe pipe)
 	struct intel_connector *intel_connector = to_intel_connector(connector);
 	struct intel_panel *panel = &intel_connector->panel;
 	int ret;
+	u32 pwm;
 
 	if (!dev_priv->vbt.backlight.present) {
 		if (dev_priv->quirks & QUIRK_BACKLIGHT_PRESENT) {
@@ -1812,6 +1819,16 @@ int intel_panel_setup_backlight(struct drm_connector *connector, enum pipe pipe)
 	/* set level and max in panel struct */
 	mutex_lock(&dev_priv->backlight_lock);
 	ret = panel->backlight.setup(intel_connector, pipe);
+	if (i915_modparams.backlight_freq) {
+		if (panel->backlight.hz_to_pwm &&
+				(pwm = panel->backlight.hz_to_pwm(intel_connector, i915_modparams.backlight_freq))) {
+			panel->backlight.max = pwm;
+			DRM_DEBUG_KMS("Override backlight frequency %u Hz\n",
+							i915_modparams.backlight_freq);
+		} else {
+			DRM_DEBUG_KMS("failed to override backlight frequency\n");
+		}
+	}
 	mutex_unlock(&dev_priv->backlight_lock);
 
 	if (ret) {
-- 
2.15.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Allow user to override PWM backlight frequency and duty cycle (rev2)
  2018-01-16 13:40 [PATCH] drm/i915: Allow user to override PWM backlight frequency and duty cycle Alex Ivanov
  2018-01-16 14:41 ` ✗ Fi.CI.BAT: failure for " Patchwork
  2018-01-17  7:22 ` [PATCH] " Alex Ivanov
@ 2018-01-17  7:46 ` Patchwork
  2018-01-17  8:59 ` ✗ Fi.CI.IGT: warning " Patchwork
  2018-01-17 13:12 ` [PATCH] drm/i915: Allow user to override PWM backlight frequency and duty cycle Jani Nikula
  4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-01-17  7:46 UTC (permalink / raw)
  To: Alex Ivanov; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Allow user to override PWM backlight frequency and duty cycle (rev2)
URL   : https://patchwork.freedesktop.org/series/36540/
State : success

== Summary ==

Series 36540v2 drm/i915: Allow user to override PWM backlight frequency and duty cycle
https://patchwork.freedesktop.org/api/1.0/series/36540/revisions/2/mbox/

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:419s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:425s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:374s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:484s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:283s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:485s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:483s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:464s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:461s
fi-elk-e7500     total:224  pass:168  dwarn:10  dfail:0   fail:0   skip:45 
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:276s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:512s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:390s
fi-hsw-4770r     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:398s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:411s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:456s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:411s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:457s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:498s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:452s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:505s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:573s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:436s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:510s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:530s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:486s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:475s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:430s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:515s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:393s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:573s
fi-glk-dsi       total:288  pass:257  dwarn:0   dfail:0   fail:1   skip:30  time:483s

bf4fa7c9c108a7c8f9a966f940d8df733b4a4b89 drm-tip: 2018y-01m-17d-07h-03m-28s UTC integration manifest
c412cd65075d drm/i915: Allow user to override PWM backlight frequency and duty cycle

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7691/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* ✗ Fi.CI.IGT: warning for drm/i915: Allow user to override PWM backlight frequency and duty cycle (rev2)
  2018-01-16 13:40 [PATCH] drm/i915: Allow user to override PWM backlight frequency and duty cycle Alex Ivanov
                   ` (2 preceding siblings ...)
  2018-01-17  7:46 ` ✓ Fi.CI.BAT: success for drm/i915: Allow user to override PWM backlight frequency and duty cycle (rev2) Patchwork
@ 2018-01-17  8:59 ` Patchwork
  2018-01-17 13:12 ` [PATCH] drm/i915: Allow user to override PWM backlight frequency and duty cycle Jani Nikula
  4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2018-01-17  8:59 UTC (permalink / raw)
  To: Alex Ivanov; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Allow user to override PWM backlight frequency and duty cycle (rev2)
URL   : https://patchwork.freedesktop.org/series/36540/
State : warning

== Summary ==

Warning: bzip Patchwork_7691/shard-snb5/results0.json.bz2 wasn't in correct JSON format
Test gem_eio:
        Subgroup in-flight:
                pass       -> DMESG-WARN (shard-snb) fdo#104058
Test gem_softpin:
        Subgroup noreloc-s3:
                pass       -> SKIP       (shard-snb) fdo#103375
                fail       -> SKIP       (shard-hsw) fdo#103540
Test pm_rc6_residency:
        Subgroup rc6-accuracy:
                pass       -> SKIP       (shard-snb)
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-offscren-pri-shrfb-draw-render:
                fail       -> PASS       (shard-snb) fdo#101623

fdo#104058 https://bugs.freedesktop.org/show_bug.cgi?id=104058
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623

shard-hsw        total:2729 pass:1547 dwarn:1   dfail:0   fail:11  skip:1170 time:8986s
shard-snb        total:2652 pass:1283 dwarn:2   dfail:0   fail:10  skip:1357 time:7618s
Blacklisted hosts:
shard-apl        total:2630 pass:1634 dwarn:1   dfail:0   fail:23  skip:971 time:13086s
shard-kbl        total:2726 pass:1800 dwarn:16  dfail:2   fail:26  skip:881 time:10508s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7691/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915: Allow user to override PWM backlight frequency and duty cycle
  2018-01-16 13:40 [PATCH] drm/i915: Allow user to override PWM backlight frequency and duty cycle Alex Ivanov
                   ` (3 preceding siblings ...)
  2018-01-17  8:59 ` ✗ Fi.CI.IGT: warning " Patchwork
@ 2018-01-17 13:12 ` Jani Nikula
  2018-01-17 17:49   ` Alex Ivanov
  4 siblings, 1 reply; 9+ messages in thread
From: Jani Nikula @ 2018-01-17 13:12 UTC (permalink / raw)
  To: Alex Ivanov, intel-gfx

On Tue, 16 Jan 2018, Alex Ivanov <gnidorah@ya.ru> wrote:
> Since vendor may set bad defaults or user just generally may prefer her health over
> display life.
> 1. Allow overriding of PWM frequency otherwise people use solutions like
> http://devbraindom.blogspot.ru/2013/03/eliminate-led-screen-flicker-with-intel.html
> which are bad because
> I. frequency convertation logic varies from chip to chip, so people set totally wrong
> frequences in most cases
> II. writing to backlight control registers directly goes into conflict with
> i915 driver doing the same
> 2. Allow to override minimal brightness. Even if vendor limit was totally correct,
> it may be too bright to use in night, plus the default will become most likely wrong
> now, since frequency was overriden

I'm divided. Clearly, the patch at hand is a technically better solution
than what the blog post has.

But I also think the folks over at the blog know they're directly
hacking on graphics registers, and whatever it is they're doing is not
supported. There's also warranted speculation this might be harmful to
their displays.

The modulation frequency and the minimum duty cycle have been chosen by
the OEM to work with their board design and display specs. We don't have
enough data to validate the values given by the user are within spec. If
we add this, it'll get used, it'll be expected to be supported, the
"unsafe" there will go unnoticed by folks copy-pasting the parameters
from blogs and forums, and if it breaks displays for folks, they'll be
angry at us, not at some random blog poster.

I guess damned if you do, damned if you don't.

BR,
Jani.

>
> ---
>  drivers/gpu/drm/i915/i915_params.c |  6 ++++++
>  drivers/gpu/drm/i915/i915_params.h |  2 ++
>  drivers/gpu/drm/i915/intel_panel.c | 17 +++++++++++++++++
>  3 files changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index b4faeb6aa2bd..ba37257846b0 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -190,3 +190,9 @@ i915_param_named(enable_dpcd_backlight, bool, 0600,
>  
>  i915_param_named(enable_gvt, bool, 0400,
>  	"Enable support for Intel GVT-g graphics virtualization host support(default:false)");
> +
> +i915_param_named_unsafe(backlight_freq, uint, 0400,
> +	"Override PWM backlight frequency (set in Hz, 0=ignore [default])");
> +
> +i915_param_named_unsafe(backlight_min_level, uint, 0400,
> +	"Override PWM backlight minimum level (duty cycle, 0=ignore [default])");
> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
> index c7292268ed43..7aba85db3da2 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -53,6 +53,8 @@
>  	param(int, edp_vswing, 0) \
>  	param(int, reset, 2) \
>  	param(unsigned int, inject_load_failure, 0) \
> +	param(unsigned int, backlight_freq, 0) \
> +	param(unsigned int, backlight_min_level, 0) \
>  	/* leave bools at the end to not create holes */ \
>  	param(bool, alpha_support, IS_ENABLED(CONFIG_DRM_I915_ALPHA_SUPPORT)) \
>  	param(bool, enable_cmd_parser, true) \
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index adc51e452e3e..f188d7c1d2de 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -1479,6 +1479,12 @@ static u32 get_backlight_min_vbt(struct intel_connector *connector)
>  
>  	WARN_ON(panel->backlight.max == 0);
>  
> +	if (i915_modparams.backlight_min_level) {
> +		DRM_DEBUG_KMS("Override backlight duty cycle %u\n",
> +						i915_modparams.backlight_min_level);
> +		return i915_modparams.backlight_min_level;
> +	}
> +
>  	/*
>  	 * XXX: If the vbt value is 255, it makes min equal to max, which leads
>  	 * to problems. There are such machines out there. Either our
> @@ -1795,6 +1801,7 @@ int intel_panel_setup_backlight(struct drm_connector *connector, enum pipe pipe)
>  	struct intel_connector *intel_connector = to_intel_connector(connector);
>  	struct intel_panel *panel = &intel_connector->panel;
>  	int ret;
> +	u32 pwm;
>  
>  	if (!dev_priv->vbt.backlight.present) {
>  		if (dev_priv->quirks & QUIRK_BACKLIGHT_PRESENT) {
> @@ -1812,6 +1819,16 @@ int intel_panel_setup_backlight(struct drm_connector *connector, enum pipe pipe)
>  	/* set level and max in panel struct */
>  	mutex_lock(&dev_priv->backlight_lock);
>  	ret = panel->backlight.setup(intel_connector, pipe);
> +	if (i915_modparams.backlight_freq) {
> +		if (panel->backlight.hz_to_pwm &&
> +				(pwm = panel->backlight.hz_to_pwm(intel_connector, i915_modparams.backlight_freq))) {
> +			panel->backlight.max = pwm;
> +			DRM_DEBUG_KMS("Override backlight frequency %u Hz\n",
> +							i915_modparams.backlight_freq);
> +		} else {
> +			DRM_DEBUG_KMS("failed to override backlight frequency\n");
> +		}
> +	}
>  	mutex_unlock(&dev_priv->backlight_lock);
>  
>  	if (ret) {

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915: Allow user to override PWM backlight frequency and duty cycle
  2018-01-17 13:12 ` [PATCH] drm/i915: Allow user to override PWM backlight frequency and duty cycle Jani Nikula
@ 2018-01-17 17:49   ` Alex Ivanov
  0 siblings, 0 replies; 9+ messages in thread
From: Alex Ivanov @ 2018-01-17 17:49 UTC (permalink / raw)
  To: Jani Nikula, intel-gfx

17.01.2018, 16:13, "Jani Nikula" <jani.nikula@linux.intel.com>:
> I'm divided. Clearly, the patch at hand is a technically better solution
> than what the blog post has.
>
> But I also think the folks over at the blog know they're directly
> hacking on graphics registers, and whatever it is they're doing is not
> supported. There's also warranted speculation this might be harmful to
> their displays.

But those using such calculators and guides don't know what precisely
they are doing, because authors only provided setup logic for *their own*
hardware, and not for other chips. This leads to more harm than it should
would be there an official way for doing overrides.

> The modulation frequency and the minimum duty cycle have been chosen by
> the OEM to work with their board design and display specs. 

Unfortunately some vendors tend to use "whatever defaults" that are not good
for humans. That's the reason why people poke with PWM registers.

> We don't have enough data to validate the values given by the user are within spec. If
> we add this, it'll get used, it'll be expected to be supported, the
> "unsafe" there will go unnoticed by folks copy-pasting the parameters
> from blogs and forums

I can rename the knobs into e.g. 
"debug_backlight_freq" and "debug_backlight_min_level" and add warnings to
descriptions so there will be FAT warnings.

> and if it breaks displays for folks, they'll be
> angry at us, not at some random blog poster.

The license on i915 driver says that its provided "as is" without any warranty.
With added warnings on kernel parameters people MUST know what they're
doing and take all responsibility.

>
> I guess damned if you do, damned if you don't.
>
> BR,
> Jani.
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-01-17 17:49 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-16 13:40 [PATCH] drm/i915: Allow user to override PWM backlight frequency and duty cycle Alex Ivanov
2018-01-16 14:41 ` ✗ Fi.CI.BAT: failure for " Patchwork
2018-01-16 15:36   ` gnidorah
2018-01-16 19:26     ` Saarinen, Jani
2018-01-17  7:22 ` [PATCH] " Alex Ivanov
2018-01-17  7:46 ` ✓ Fi.CI.BAT: success for drm/i915: Allow user to override PWM backlight frequency and duty cycle (rev2) Patchwork
2018-01-17  8:59 ` ✗ Fi.CI.IGT: warning " Patchwork
2018-01-17 13:12 ` [PATCH] drm/i915: Allow user to override PWM backlight frequency and duty cycle Jani Nikula
2018-01-17 17:49   ` Alex Ivanov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.