All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] drm/amd/display: some backlight fixes
@ 2021-02-03 12:42 ` Takashi Iwai
  0 siblings, 0 replies; 16+ messages in thread
From: Takashi Iwai @ 2021-02-03 12:42 UTC (permalink / raw)
  To: Alex Deucher, Christian König; +Cc: Leo Li, dri-devel, amd-gfx

Hi,

here are two patches handling the issues in the backlight control.
The first one is to fix the sysfs brightness read for devices with the
backlight controlled by aux channel.  The second one is to add a new
module option, aux_backlight, to forcibly enable/disable the aux
channel backlight control.  It's no direct solution for the bug we've
hit, but it gives at least a workaround.

BugLink: https://bugzilla.opensuse.org/show_bug.cgi?id=1180749
BugLink: https://gitlab.freedesktop.org/drm/amd/-/issues/1438


thanks,

Takashi

===

Takashi Iwai (2):
  drm/amd/display: Fix the brightness read via aux
  drm/amd/display: Add aux_backlight module option

 drivers/gpu/drm/amd/amdgpu/amdgpu.h           |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c       |  4 ++++
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 20 ++++++++++++++++++-
 3 files changed, 24 insertions(+), 1 deletion(-)

-- 
2.26.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH 0/2] drm/amd/display: some backlight fixes
@ 2021-02-03 12:42 ` Takashi Iwai
  0 siblings, 0 replies; 16+ messages in thread
From: Takashi Iwai @ 2021-02-03 12:42 UTC (permalink / raw)
  To: Alex Deucher, Christian König
  Cc: Leo Li, Harry Wentland, dri-devel, amd-gfx

Hi,

here are two patches handling the issues in the backlight control.
The first one is to fix the sysfs brightness read for devices with the
backlight controlled by aux channel.  The second one is to add a new
module option, aux_backlight, to forcibly enable/disable the aux
channel backlight control.  It's no direct solution for the bug we've
hit, but it gives at least a workaround.

BugLink: https://bugzilla.opensuse.org/show_bug.cgi?id=1180749
BugLink: https://gitlab.freedesktop.org/drm/amd/-/issues/1438


thanks,

Takashi

===

Takashi Iwai (2):
  drm/amd/display: Fix the brightness read via aux
  drm/amd/display: Add aux_backlight module option

 drivers/gpu/drm/amd/amdgpu/amdgpu.h           |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c       |  4 ++++
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 20 ++++++++++++++++++-
 3 files changed, 24 insertions(+), 1 deletion(-)

-- 
2.26.2

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

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

* [PATCH 1/2] drm/amd/display: Fix the brightness read via aux
  2021-02-03 12:42 ` Takashi Iwai
@ 2021-02-03 12:42   ` Takashi Iwai
  -1 siblings, 0 replies; 16+ messages in thread
From: Takashi Iwai @ 2021-02-03 12:42 UTC (permalink / raw)
  To: Alex Deucher, Christian König; +Cc: Leo Li, dri-devel, amd-gfx

The current code tries to read the brightness value via
dc_link_get_backlight_level() no matter whether it's controlled via
aux or not, and this results in a bogus value returned.
Fix it to read the current value via
dc_link_get_backlight_level_nits() for the aux.

BugLink: https://bugzilla.opensuse.org/show_bug.cgi?id=1180749
BugLink: https://gitlab.freedesktop.org/drm/amd/-/issues/1438
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index c6da89df055d..fb62886ae013 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3140,6 +3140,16 @@ static int set_backlight_via_aux(struct dc_link *link, uint32_t brightness)
 	return rc ? 0 : 1;
 }
 
+static int get_backlight_via_aux(struct dc_link *link)
+{
+	uint32_t avg, peak;
+
+	if (!link ||
+	    !dc_link_get_backlight_level_nits(link, &avg, &peak))
+		return DC_ERROR_UNEXPECTED;
+	return avg;
+}
+
 static int get_brightness_range(const struct amdgpu_dm_backlight_caps *caps,
 				unsigned *min, unsigned *max)
 {
@@ -3212,8 +3222,13 @@ static int amdgpu_dm_backlight_update_status(struct backlight_device *bd)
 static int amdgpu_dm_backlight_get_brightness(struct backlight_device *bd)
 {
 	struct amdgpu_display_manager *dm = bl_get_data(bd);
-	int ret = dc_link_get_backlight_level(dm->backlight_link);
+	struct dc_link *link = (struct dc_link *)dm->backlight_link;
+	int ret;
 
+	if (dm->backlight_caps.aux_support)
+		ret = get_backlight_via_aux(link);
+	else
+		ret = dc_link_get_backlight_level(link);
 	if (ret == DC_ERROR_UNEXPECTED)
 		return bd->props.brightness;
 	return convert_brightness_to_user(&dm->backlight_caps, ret);
-- 
2.26.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH 1/2] drm/amd/display: Fix the brightness read via aux
@ 2021-02-03 12:42   ` Takashi Iwai
  0 siblings, 0 replies; 16+ messages in thread
From: Takashi Iwai @ 2021-02-03 12:42 UTC (permalink / raw)
  To: Alex Deucher, Christian König
  Cc: Leo Li, Harry Wentland, dri-devel, amd-gfx

The current code tries to read the brightness value via
dc_link_get_backlight_level() no matter whether it's controlled via
aux or not, and this results in a bogus value returned.
Fix it to read the current value via
dc_link_get_backlight_level_nits() for the aux.

BugLink: https://bugzilla.opensuse.org/show_bug.cgi?id=1180749
BugLink: https://gitlab.freedesktop.org/drm/amd/-/issues/1438
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index c6da89df055d..fb62886ae013 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3140,6 +3140,16 @@ static int set_backlight_via_aux(struct dc_link *link, uint32_t brightness)
 	return rc ? 0 : 1;
 }
 
+static int get_backlight_via_aux(struct dc_link *link)
+{
+	uint32_t avg, peak;
+
+	if (!link ||
+	    !dc_link_get_backlight_level_nits(link, &avg, &peak))
+		return DC_ERROR_UNEXPECTED;
+	return avg;
+}
+
 static int get_brightness_range(const struct amdgpu_dm_backlight_caps *caps,
 				unsigned *min, unsigned *max)
 {
@@ -3212,8 +3222,13 @@ static int amdgpu_dm_backlight_update_status(struct backlight_device *bd)
 static int amdgpu_dm_backlight_get_brightness(struct backlight_device *bd)
 {
 	struct amdgpu_display_manager *dm = bl_get_data(bd);
-	int ret = dc_link_get_backlight_level(dm->backlight_link);
+	struct dc_link *link = (struct dc_link *)dm->backlight_link;
+	int ret;
 
+	if (dm->backlight_caps.aux_support)
+		ret = get_backlight_via_aux(link);
+	else
+		ret = dc_link_get_backlight_level(link);
 	if (ret == DC_ERROR_UNEXPECTED)
 		return bd->props.brightness;
 	return convert_brightness_to_user(&dm->backlight_caps, ret);
-- 
2.26.2

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

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

* [PATCH 2/2] drm/amd/display: Add aux_backlight module option
  2021-02-03 12:42 ` Takashi Iwai
@ 2021-02-03 12:42   ` Takashi Iwai
  -1 siblings, 0 replies; 16+ messages in thread
From: Takashi Iwai @ 2021-02-03 12:42 UTC (permalink / raw)
  To: Alex Deucher, Christian König; +Cc: Leo Li, dri-devel, amd-gfx

There seem devices that don't work with the aux channel backlight
control.  For allowing such users to test with the other backlight
control method, provide a new module option, aux_backlight, to specify
enabling or disabling the aux backport support explicitly.  As
default, the aux support is detected by the hardware capability.

BugLink: https://bugzilla.opensuse.org/show_bug.cgi?id=1180749
BugLink: https://gitlab.freedesktop.org/drm/amd/-/issues/1438
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h               | 1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c           | 4 ++++
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +++
 3 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 5993dd0fdd8e..4793cd5e69f9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -179,6 +179,7 @@ extern uint amdgpu_smu_memory_pool_size;
 extern uint amdgpu_dc_feature_mask;
 extern uint amdgpu_dc_debug_mask;
 extern uint amdgpu_dm_abm_level;
+extern int amdgpu_aux_backlight;
 extern struct amdgpu_mgpu_info mgpu_info;
 extern int amdgpu_ras_enable;
 extern uint amdgpu_ras_mask;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 7169fb5e3d9c..5b66822da954 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -777,6 +777,10 @@ uint amdgpu_dm_abm_level;
 MODULE_PARM_DESC(abmlevel, "ABM level (0 = off (default), 1-4 = backlight reduction level) ");
 module_param_named(abmlevel, amdgpu_dm_abm_level, uint, 0444);
 
+int amdgpu_aux_backlight = -1;
+MODULE_PARM_DESC(aux_backlight, "Aux backlight control (0 = off, 1 = on, default auto)");
+module_param_named(aux_backlight, amdgpu_aux_backlight, bint, 0444);
+
 /**
  * DOC: tmz (int)
  * Trusted Memory Zone (TMZ) is a method to protect data being written
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index fb62886ae013..6ad384ef61b8 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2209,6 +2209,9 @@ static void update_connector_ext_caps(struct amdgpu_dm_connector *aconnector)
 	    caps->ext_caps->bits.hdr_aux_backlight_control == 1)
 		caps->aux_support = true;
 
+	if (amdgpu_aux_backlight >= 0)
+		caps->aux_support = amdgpu_aux_backlight;
+
 	/* From the specification (CTA-861-G), for calculating the maximum
 	 * luminance we need to use:
 	 *	Luminance = 50*2**(CV/32)
-- 
2.26.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH 2/2] drm/amd/display: Add aux_backlight module option
@ 2021-02-03 12:42   ` Takashi Iwai
  0 siblings, 0 replies; 16+ messages in thread
From: Takashi Iwai @ 2021-02-03 12:42 UTC (permalink / raw)
  To: Alex Deucher, Christian König
  Cc: Leo Li, Harry Wentland, dri-devel, amd-gfx

There seem devices that don't work with the aux channel backlight
control.  For allowing such users to test with the other backlight
control method, provide a new module option, aux_backlight, to specify
enabling or disabling the aux backport support explicitly.  As
default, the aux support is detected by the hardware capability.

BugLink: https://bugzilla.opensuse.org/show_bug.cgi?id=1180749
BugLink: https://gitlab.freedesktop.org/drm/amd/-/issues/1438
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h               | 1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c           | 4 ++++
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +++
 3 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 5993dd0fdd8e..4793cd5e69f9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -179,6 +179,7 @@ extern uint amdgpu_smu_memory_pool_size;
 extern uint amdgpu_dc_feature_mask;
 extern uint amdgpu_dc_debug_mask;
 extern uint amdgpu_dm_abm_level;
+extern int amdgpu_aux_backlight;
 extern struct amdgpu_mgpu_info mgpu_info;
 extern int amdgpu_ras_enable;
 extern uint amdgpu_ras_mask;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 7169fb5e3d9c..5b66822da954 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -777,6 +777,10 @@ uint amdgpu_dm_abm_level;
 MODULE_PARM_DESC(abmlevel, "ABM level (0 = off (default), 1-4 = backlight reduction level) ");
 module_param_named(abmlevel, amdgpu_dm_abm_level, uint, 0444);
 
+int amdgpu_aux_backlight = -1;
+MODULE_PARM_DESC(aux_backlight, "Aux backlight control (0 = off, 1 = on, default auto)");
+module_param_named(aux_backlight, amdgpu_aux_backlight, bint, 0444);
+
 /**
  * DOC: tmz (int)
  * Trusted Memory Zone (TMZ) is a method to protect data being written
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index fb62886ae013..6ad384ef61b8 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2209,6 +2209,9 @@ static void update_connector_ext_caps(struct amdgpu_dm_connector *aconnector)
 	    caps->ext_caps->bits.hdr_aux_backlight_control == 1)
 		caps->aux_support = true;
 
+	if (amdgpu_aux_backlight >= 0)
+		caps->aux_support = amdgpu_aux_backlight;
+
 	/* From the specification (CTA-861-G), for calculating the maximum
 	 * luminance we need to use:
 	 *	Luminance = 50*2**(CV/32)
-- 
2.26.2

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

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

* Re: [PATCH 2/2] drm/amd/display: Add aux_backlight module option
  2021-02-03 12:42   ` Takashi Iwai
@ 2021-02-05 16:34     ` Alex Deucher
  -1 siblings, 0 replies; 16+ messages in thread
From: Alex Deucher @ 2021-02-05 16:34 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Leo Li, amd-gfx list, Maling list - DRI developers, Alex Deucher,
	Christian König

On Wed, Feb 3, 2021 at 7:42 AM Takashi Iwai <tiwai@suse.de> wrote:
>
> There seem devices that don't work with the aux channel backlight
> control.  For allowing such users to test with the other backlight
> control method, provide a new module option, aux_backlight, to specify
> enabling or disabling the aux backport support explicitly.  As
> default, the aux support is detected by the hardware capability.
>
> BugLink: https://bugzilla.opensuse.org/show_bug.cgi?id=1180749
> BugLink: https://gitlab.freedesktop.org/drm/amd/-/issues/1438
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h               | 1 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c           | 4 ++++
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +++
>  3 files changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 5993dd0fdd8e..4793cd5e69f9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -179,6 +179,7 @@ extern uint amdgpu_smu_memory_pool_size;
>  extern uint amdgpu_dc_feature_mask;
>  extern uint amdgpu_dc_debug_mask;
>  extern uint amdgpu_dm_abm_level;
> +extern int amdgpu_aux_backlight;
>  extern struct amdgpu_mgpu_info mgpu_info;
>  extern int amdgpu_ras_enable;
>  extern uint amdgpu_ras_mask;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 7169fb5e3d9c..5b66822da954 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -777,6 +777,10 @@ uint amdgpu_dm_abm_level;
>  MODULE_PARM_DESC(abmlevel, "ABM level (0 = off (default), 1-4 = backlight reduction level) ");
>  module_param_named(abmlevel, amdgpu_dm_abm_level, uint, 0444);
>
> +int amdgpu_aux_backlight = -1;
> +MODULE_PARM_DESC(aux_backlight, "Aux backlight control (0 = off, 1 = on, default auto)");
> +module_param_named(aux_backlight, amdgpu_aux_backlight, bint, 0444);

I'd suggest making this something more generic like "backlight" and
make -1 auto, 0 pwm, 1 aux.  That way we can handle potential future
types more cleanly.

Alex


> +
>  /**
>   * DOC: tmz (int)
>   * Trusted Memory Zone (TMZ) is a method to protect data being written
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index fb62886ae013..6ad384ef61b8 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2209,6 +2209,9 @@ static void update_connector_ext_caps(struct amdgpu_dm_connector *aconnector)
>             caps->ext_caps->bits.hdr_aux_backlight_control == 1)
>                 caps->aux_support = true;
>
> +       if (amdgpu_aux_backlight >= 0)
> +               caps->aux_support = amdgpu_aux_backlight;
> +
>         /* From the specification (CTA-861-G), for calculating the maximum
>          * luminance we need to use:
>          *      Luminance = 50*2**(CV/32)
> --
> 2.26.2
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 2/2] drm/amd/display: Add aux_backlight module option
@ 2021-02-05 16:34     ` Alex Deucher
  0 siblings, 0 replies; 16+ messages in thread
From: Alex Deucher @ 2021-02-05 16:34 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Leo Li, amd-gfx list, Maling list - DRI developers, Alex Deucher,
	Harry Wentland, Christian König

On Wed, Feb 3, 2021 at 7:42 AM Takashi Iwai <tiwai@suse.de> wrote:
>
> There seem devices that don't work with the aux channel backlight
> control.  For allowing such users to test with the other backlight
> control method, provide a new module option, aux_backlight, to specify
> enabling or disabling the aux backport support explicitly.  As
> default, the aux support is detected by the hardware capability.
>
> BugLink: https://bugzilla.opensuse.org/show_bug.cgi?id=1180749
> BugLink: https://gitlab.freedesktop.org/drm/amd/-/issues/1438
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h               | 1 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c           | 4 ++++
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +++
>  3 files changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 5993dd0fdd8e..4793cd5e69f9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -179,6 +179,7 @@ extern uint amdgpu_smu_memory_pool_size;
>  extern uint amdgpu_dc_feature_mask;
>  extern uint amdgpu_dc_debug_mask;
>  extern uint amdgpu_dm_abm_level;
> +extern int amdgpu_aux_backlight;
>  extern struct amdgpu_mgpu_info mgpu_info;
>  extern int amdgpu_ras_enable;
>  extern uint amdgpu_ras_mask;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 7169fb5e3d9c..5b66822da954 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -777,6 +777,10 @@ uint amdgpu_dm_abm_level;
>  MODULE_PARM_DESC(abmlevel, "ABM level (0 = off (default), 1-4 = backlight reduction level) ");
>  module_param_named(abmlevel, amdgpu_dm_abm_level, uint, 0444);
>
> +int amdgpu_aux_backlight = -1;
> +MODULE_PARM_DESC(aux_backlight, "Aux backlight control (0 = off, 1 = on, default auto)");
> +module_param_named(aux_backlight, amdgpu_aux_backlight, bint, 0444);

I'd suggest making this something more generic like "backlight" and
make -1 auto, 0 pwm, 1 aux.  That way we can handle potential future
types more cleanly.

Alex


> +
>  /**
>   * DOC: tmz (int)
>   * Trusted Memory Zone (TMZ) is a method to protect data being written
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index fb62886ae013..6ad384ef61b8 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2209,6 +2209,9 @@ static void update_connector_ext_caps(struct amdgpu_dm_connector *aconnector)
>             caps->ext_caps->bits.hdr_aux_backlight_control == 1)
>                 caps->aux_support = true;
>
> +       if (amdgpu_aux_backlight >= 0)
> +               caps->aux_support = amdgpu_aux_backlight;
> +
>         /* From the specification (CTA-861-G), for calculating the maximum
>          * luminance we need to use:
>          *      Luminance = 50*2**(CV/32)
> --
> 2.26.2
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

* Re: [PATCH 1/2] drm/amd/display: Fix the brightness read via aux
  2021-02-03 12:42   ` Takashi Iwai
@ 2021-02-05 16:36     ` Alex Deucher
  -1 siblings, 0 replies; 16+ messages in thread
From: Alex Deucher @ 2021-02-05 16:36 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Leo Li, amd-gfx list, Maling list - DRI developers, Alex Deucher,
	Christian König

On Wed, Feb 3, 2021 at 7:42 AM Takashi Iwai <tiwai@suse.de> wrote:
>
> The current code tries to read the brightness value via
> dc_link_get_backlight_level() no matter whether it's controlled via
> aux or not, and this results in a bogus value returned.
> Fix it to read the current value via
> dc_link_get_backlight_level_nits() for the aux.
>
> BugLink: https://bugzilla.opensuse.org/show_bug.cgi?id=1180749
> BugLink: https://gitlab.freedesktop.org/drm/amd/-/issues/1438
> Signed-off-by: Takashi Iwai <tiwai@suse.de>

This looks fine to me.  FWIW, I have a similar patch set here:
https://cgit.freedesktop.org/~agd5f/linux/log/?h=backlight_wip

Alex

> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index c6da89df055d..fb62886ae013 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -3140,6 +3140,16 @@ static int set_backlight_via_aux(struct dc_link *link, uint32_t brightness)
>         return rc ? 0 : 1;
>  }
>
> +static int get_backlight_via_aux(struct dc_link *link)
> +{
> +       uint32_t avg, peak;
> +
> +       if (!link ||
> +           !dc_link_get_backlight_level_nits(link, &avg, &peak))
> +               return DC_ERROR_UNEXPECTED;
> +       return avg;
> +}
> +
>  static int get_brightness_range(const struct amdgpu_dm_backlight_caps *caps,
>                                 unsigned *min, unsigned *max)
>  {
> @@ -3212,8 +3222,13 @@ static int amdgpu_dm_backlight_update_status(struct backlight_device *bd)
>  static int amdgpu_dm_backlight_get_brightness(struct backlight_device *bd)
>  {
>         struct amdgpu_display_manager *dm = bl_get_data(bd);
> -       int ret = dc_link_get_backlight_level(dm->backlight_link);
> +       struct dc_link *link = (struct dc_link *)dm->backlight_link;
> +       int ret;
>
> +       if (dm->backlight_caps.aux_support)
> +               ret = get_backlight_via_aux(link);
> +       else
> +               ret = dc_link_get_backlight_level(link);
>         if (ret == DC_ERROR_UNEXPECTED)
>                 return bd->props.brightness;
>         return convert_brightness_to_user(&dm->backlight_caps, ret);
> --
> 2.26.2
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 1/2] drm/amd/display: Fix the brightness read via aux
@ 2021-02-05 16:36     ` Alex Deucher
  0 siblings, 0 replies; 16+ messages in thread
From: Alex Deucher @ 2021-02-05 16:36 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Leo Li, amd-gfx list, Maling list - DRI developers, Alex Deucher,
	Harry Wentland, Christian König

On Wed, Feb 3, 2021 at 7:42 AM Takashi Iwai <tiwai@suse.de> wrote:
>
> The current code tries to read the brightness value via
> dc_link_get_backlight_level() no matter whether it's controlled via
> aux or not, and this results in a bogus value returned.
> Fix it to read the current value via
> dc_link_get_backlight_level_nits() for the aux.
>
> BugLink: https://bugzilla.opensuse.org/show_bug.cgi?id=1180749
> BugLink: https://gitlab.freedesktop.org/drm/amd/-/issues/1438
> Signed-off-by: Takashi Iwai <tiwai@suse.de>

This looks fine to me.  FWIW, I have a similar patch set here:
https://cgit.freedesktop.org/~agd5f/linux/log/?h=backlight_wip

Alex

> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c   | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index c6da89df055d..fb62886ae013 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -3140,6 +3140,16 @@ static int set_backlight_via_aux(struct dc_link *link, uint32_t brightness)
>         return rc ? 0 : 1;
>  }
>
> +static int get_backlight_via_aux(struct dc_link *link)
> +{
> +       uint32_t avg, peak;
> +
> +       if (!link ||
> +           !dc_link_get_backlight_level_nits(link, &avg, &peak))
> +               return DC_ERROR_UNEXPECTED;
> +       return avg;
> +}
> +
>  static int get_brightness_range(const struct amdgpu_dm_backlight_caps *caps,
>                                 unsigned *min, unsigned *max)
>  {
> @@ -3212,8 +3222,13 @@ static int amdgpu_dm_backlight_update_status(struct backlight_device *bd)
>  static int amdgpu_dm_backlight_get_brightness(struct backlight_device *bd)
>  {
>         struct amdgpu_display_manager *dm = bl_get_data(bd);
> -       int ret = dc_link_get_backlight_level(dm->backlight_link);
> +       struct dc_link *link = (struct dc_link *)dm->backlight_link;
> +       int ret;
>
> +       if (dm->backlight_caps.aux_support)
> +               ret = get_backlight_via_aux(link);
> +       else
> +               ret = dc_link_get_backlight_level(link);
>         if (ret == DC_ERROR_UNEXPECTED)
>                 return bd->props.brightness;
>         return convert_brightness_to_user(&dm->backlight_caps, ret);
> --
> 2.26.2
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

* Re: [PATCH 2/2] drm/amd/display: Add aux_backlight module option
  2021-02-05 16:34     ` Alex Deucher
@ 2021-02-06 12:25       ` Takashi Iwai
  -1 siblings, 0 replies; 16+ messages in thread
From: Takashi Iwai @ 2021-02-06 12:25 UTC (permalink / raw)
  To: Alex Deucher
  Cc: Leo Li, amd-gfx list, Maling list - DRI developers, Alex Deucher,
	Christian König

On Fri, 05 Feb 2021 17:34:36 +0100,
Alex Deucher wrote:
> 
> On Wed, Feb 3, 2021 at 7:42 AM Takashi Iwai <tiwai@suse.de> wrote:
> >
> > There seem devices that don't work with the aux channel backlight
> > control.  For allowing such users to test with the other backlight
> > control method, provide a new module option, aux_backlight, to specify
> > enabling or disabling the aux backport support explicitly.  As
> > default, the aux support is detected by the hardware capability.
> >
> > BugLink: https://bugzilla.opensuse.org/show_bug.cgi?id=1180749
> > BugLink: https://gitlab.freedesktop.org/drm/amd/-/issues/1438
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu.h               | 1 +
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c           | 4 ++++
> >  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +++
> >  3 files changed, 8 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > index 5993dd0fdd8e..4793cd5e69f9 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > @@ -179,6 +179,7 @@ extern uint amdgpu_smu_memory_pool_size;
> >  extern uint amdgpu_dc_feature_mask;
> >  extern uint amdgpu_dc_debug_mask;
> >  extern uint amdgpu_dm_abm_level;
> > +extern int amdgpu_aux_backlight;
> >  extern struct amdgpu_mgpu_info mgpu_info;
> >  extern int amdgpu_ras_enable;
> >  extern uint amdgpu_ras_mask;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > index 7169fb5e3d9c..5b66822da954 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > @@ -777,6 +777,10 @@ uint amdgpu_dm_abm_level;
> >  MODULE_PARM_DESC(abmlevel, "ABM level (0 = off (default), 1-4 = backlight reduction level) ");
> >  module_param_named(abmlevel, amdgpu_dm_abm_level, uint, 0444);
> >
> > +int amdgpu_aux_backlight = -1;
> > +MODULE_PARM_DESC(aux_backlight, "Aux backlight control (0 = off, 1 = on, default auto)");
> > +module_param_named(aux_backlight, amdgpu_aux_backlight, bint, 0444);
> 
> I'd suggest making this something more generic like "backlight" and
> make -1 auto, 0 pwm, 1 aux.  That way we can handle potential future
> types more cleanly.

OK, will respin later.


thanks,

Takashi
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 2/2] drm/amd/display: Add aux_backlight module option
@ 2021-02-06 12:25       ` Takashi Iwai
  0 siblings, 0 replies; 16+ messages in thread
From: Takashi Iwai @ 2021-02-06 12:25 UTC (permalink / raw)
  To: Alex Deucher
  Cc: Leo Li, amd-gfx list, Maling list - DRI developers, Alex Deucher,
	Harry Wentland, Christian König

On Fri, 05 Feb 2021 17:34:36 +0100,
Alex Deucher wrote:
> 
> On Wed, Feb 3, 2021 at 7:42 AM Takashi Iwai <tiwai@suse.de> wrote:
> >
> > There seem devices that don't work with the aux channel backlight
> > control.  For allowing such users to test with the other backlight
> > control method, provide a new module option, aux_backlight, to specify
> > enabling or disabling the aux backport support explicitly.  As
> > default, the aux support is detected by the hardware capability.
> >
> > BugLink: https://bugzilla.opensuse.org/show_bug.cgi?id=1180749
> > BugLink: https://gitlab.freedesktop.org/drm/amd/-/issues/1438
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu.h               | 1 +
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c           | 4 ++++
> >  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +++
> >  3 files changed, 8 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > index 5993dd0fdd8e..4793cd5e69f9 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > @@ -179,6 +179,7 @@ extern uint amdgpu_smu_memory_pool_size;
> >  extern uint amdgpu_dc_feature_mask;
> >  extern uint amdgpu_dc_debug_mask;
> >  extern uint amdgpu_dm_abm_level;
> > +extern int amdgpu_aux_backlight;
> >  extern struct amdgpu_mgpu_info mgpu_info;
> >  extern int amdgpu_ras_enable;
> >  extern uint amdgpu_ras_mask;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > index 7169fb5e3d9c..5b66822da954 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > @@ -777,6 +777,10 @@ uint amdgpu_dm_abm_level;
> >  MODULE_PARM_DESC(abmlevel, "ABM level (0 = off (default), 1-4 = backlight reduction level) ");
> >  module_param_named(abmlevel, amdgpu_dm_abm_level, uint, 0444);
> >
> > +int amdgpu_aux_backlight = -1;
> > +MODULE_PARM_DESC(aux_backlight, "Aux backlight control (0 = off, 1 = on, default auto)");
> > +module_param_named(aux_backlight, amdgpu_aux_backlight, bint, 0444);
> 
> I'd suggest making this something more generic like "backlight" and
> make -1 auto, 0 pwm, 1 aux.  That way we can handle potential future
> types more cleanly.

OK, will respin later.


thanks,

Takashi
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

* Re: [PATCH 1/2] drm/amd/display: Fix the brightness read via aux
  2021-02-05 16:36     ` Alex Deucher
@ 2021-02-06 12:29       ` Takashi Iwai
  -1 siblings, 0 replies; 16+ messages in thread
From: Takashi Iwai @ 2021-02-06 12:29 UTC (permalink / raw)
  To: Alex Deucher
  Cc: Leo Li, amd-gfx list, Maling list - DRI developers, Alex Deucher,
	Christian König

On Fri, 05 Feb 2021 17:36:44 +0100,
Alex Deucher wrote:
> 
> On Wed, Feb 3, 2021 at 7:42 AM Takashi Iwai <tiwai@suse.de> wrote:
> >
> > The current code tries to read the brightness value via
> > dc_link_get_backlight_level() no matter whether it's controlled via
> > aux or not, and this results in a bogus value returned.
> > Fix it to read the current value via
> > dc_link_get_backlight_level_nits() for the aux.
> >
> > BugLink: https://bugzilla.opensuse.org/show_bug.cgi?id=1180749
> > BugLink: https://gitlab.freedesktop.org/drm/amd/-/issues/1438
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> 
> This looks fine to me.  FWIW, I have a similar patch set here:
> https://cgit.freedesktop.org/~agd5f/linux/log/?h=backlight_wip

I'm fine to scratch mine as long as the issue gets fixed :)

FWIW, the biggest problem so far was the aux channel backlight didn't
work as expected, the actual backlight isn't changed by the backlight
sysfs write.  (And the sysfs read gives a bogus value, but it's not
the cause of the non-working backlight control.)

Does the aux channel backlight really work with the current code?
Or is this rather a device-specific issue (e.g. broken BIOS) and we
might need to come up with a deny list or such?


thanks,

Takashi
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 1/2] drm/amd/display: Fix the brightness read via aux
@ 2021-02-06 12:29       ` Takashi Iwai
  0 siblings, 0 replies; 16+ messages in thread
From: Takashi Iwai @ 2021-02-06 12:29 UTC (permalink / raw)
  To: Alex Deucher
  Cc: Leo Li, amd-gfx list, Maling list - DRI developers, Alex Deucher,
	Harry Wentland, Christian König

On Fri, 05 Feb 2021 17:36:44 +0100,
Alex Deucher wrote:
> 
> On Wed, Feb 3, 2021 at 7:42 AM Takashi Iwai <tiwai@suse.de> wrote:
> >
> > The current code tries to read the brightness value via
> > dc_link_get_backlight_level() no matter whether it's controlled via
> > aux or not, and this results in a bogus value returned.
> > Fix it to read the current value via
> > dc_link_get_backlight_level_nits() for the aux.
> >
> > BugLink: https://bugzilla.opensuse.org/show_bug.cgi?id=1180749
> > BugLink: https://gitlab.freedesktop.org/drm/amd/-/issues/1438
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> 
> This looks fine to me.  FWIW, I have a similar patch set here:
> https://cgit.freedesktop.org/~agd5f/linux/log/?h=backlight_wip

I'm fine to scratch mine as long as the issue gets fixed :)

FWIW, the biggest problem so far was the aux channel backlight didn't
work as expected, the actual backlight isn't changed by the backlight
sysfs write.  (And the sysfs read gives a bogus value, but it's not
the cause of the non-working backlight control.)

Does the aux channel backlight really work with the current code?
Or is this rather a device-specific issue (e.g. broken BIOS) and we
might need to come up with a deny list or such?


thanks,

Takashi
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

* RE: [PATCH 1/2] drm/amd/display: Fix the brightness read via aux
  2021-02-06 12:29       ` Takashi Iwai
@ 2021-02-08 15:02         ` Deucher, Alexander
  -1 siblings, 0 replies; 16+ messages in thread
From: Deucher, Alexander @ 2021-02-08 15:02 UTC (permalink / raw)
  To: Takashi Iwai, Alex Deucher, Kazlauskas, Nicholas, Siqueira, Rodrigo
  Cc: Li, Sun peng (Leo),
	amd-gfx list, Koenig, Christian, Maling list - DRI developers

[AMD Public Use]

> -----Original Message-----
> From: Takashi Iwai <tiwai@suse.de>
> Sent: Saturday, February 6, 2021 7:29 AM
> To: Alex Deucher <alexdeucher@gmail.com>
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian
> <Christian.Koenig@amd.com>; Li, Sun peng (Leo) <Sunpeng.Li@amd.com>;
> Wentland, Harry <Harry.Wentland@amd.com>; Maling list - DRI developers
> <dri-devel@lists.freedesktop.org>; amd-gfx list <amd-
> gfx@lists.freedesktop.org>
> Subject: Re: [PATCH 1/2] drm/amd/display: Fix the brightness read via aux
> 
> On Fri, 05 Feb 2021 17:36:44 +0100,
> Alex Deucher wrote:
> >
> > On Wed, Feb 3, 2021 at 7:42 AM Takashi Iwai <tiwai@suse.de> wrote:
> > >
> > > The current code tries to read the brightness value via
> > > dc_link_get_backlight_level() no matter whether it's controlled via
> > > aux or not, and this results in a bogus value returned.
> > > Fix it to read the current value via
> > > dc_link_get_backlight_level_nits() for the aux.
> > >
> > > BugLink:
> > >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbu
> > >
> gzilla.opensuse.org%2Fshow_bug.cgi%3Fid%3D1180749&amp;data=04%7C01
> %7
> > >
> Calexander.deucher%40amd.com%7Ce5579cfe56f74b572f1508d8ca9ad0ac%7
> C3d
> > >
> d8961fe4884e608e11a82d994e183d%7C0%7C0%7C637482113562863043%7CU
> nknow
> > >
> n%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1ha
> Wwi
> > >
> LCJXVCI6Mn0%3D%7C1000&amp;sdata=HVtqM2r6oxSWd3XGGQZotO8wrvM
> qCTcwfq1L
> > > 2%2FeCmSE%3D&amp;reserved=0
> > > BugLink:
> > >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
> > > tlab.freedesktop.org%2Fdrm%2Famd%2F-
> %2Fissues%2F1438&amp;data=04%7C0
> > >
> 1%7Calexander.deucher%40amd.com%7Ce5579cfe56f74b572f1508d8ca9ad0
> ac%7
> > >
> C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637482113562863043%
> 7CUnk
> > >
> nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6I
> k1ha
> > >
> WwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=TdYgwNJ%2FvkuoDLNb9ATFb1P
> yznlp%2F
> > > P8TLuYSR%2BVkNqY%3D&amp;reserved=0
> > > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> >
> > This looks fine to me.  FWIW, I have a similar patch set here:
> > https://nam11.safelinks.protection.outlook.com/?url=https:%2F%2Fcgit.f
> >
> reedesktop.org%2F~agd5f%2Flinux%2Flog%2F%3Fh%3Dbacklight_wip&amp;
> data=
> >
> 04%7C01%7Calexander.deucher%40amd.com%7Ce5579cfe56f74b572f1508d8
> ca9ad0
> >
> ac%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637482113562863
> 043%7CU
> >
> nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI
> 6Ik1ha
> >
> WwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=aoMSY0nvHjrLocUPJtdgckqIH7x
> LUPbwpH0
> > ZjhuuJO8%3D&amp;reserved=0
> 
> I'm fine to scratch mine as long as the issue gets fixed :)
> 
> FWIW, the biggest problem so far was the aux channel backlight didn't work
> as expected, the actual backlight isn't changed by the backlight sysfs write.
> (And the sysfs read gives a bogus value, but it's not the cause of the non-
> working backlight control.)
> 
> Does the aux channel backlight really work with the current code?
> Or is this rather a device-specific issue (e.g. broken BIOS) and we might need
> to come up with a deny list or such?
> 

@Kazlauskas, Nicholas, @Siqueira, Rodrigo

Has there been any progress on the backlight fixes?

Alex
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* RE: [PATCH 1/2] drm/amd/display: Fix the brightness read via aux
@ 2021-02-08 15:02         ` Deucher, Alexander
  0 siblings, 0 replies; 16+ messages in thread
From: Deucher, Alexander @ 2021-02-08 15:02 UTC (permalink / raw)
  To: Takashi Iwai, Alex Deucher, Kazlauskas, Nicholas, Siqueira, Rodrigo
  Cc: Li, Sun peng (Leo),
	amd-gfx list, Wentland, Harry, Koenig, Christian,
	Maling list - DRI developers

[AMD Public Use]

> -----Original Message-----
> From: Takashi Iwai <tiwai@suse.de>
> Sent: Saturday, February 6, 2021 7:29 AM
> To: Alex Deucher <alexdeucher@gmail.com>
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian
> <Christian.Koenig@amd.com>; Li, Sun peng (Leo) <Sunpeng.Li@amd.com>;
> Wentland, Harry <Harry.Wentland@amd.com>; Maling list - DRI developers
> <dri-devel@lists.freedesktop.org>; amd-gfx list <amd-
> gfx@lists.freedesktop.org>
> Subject: Re: [PATCH 1/2] drm/amd/display: Fix the brightness read via aux
> 
> On Fri, 05 Feb 2021 17:36:44 +0100,
> Alex Deucher wrote:
> >
> > On Wed, Feb 3, 2021 at 7:42 AM Takashi Iwai <tiwai@suse.de> wrote:
> > >
> > > The current code tries to read the brightness value via
> > > dc_link_get_backlight_level() no matter whether it's controlled via
> > > aux or not, and this results in a bogus value returned.
> > > Fix it to read the current value via
> > > dc_link_get_backlight_level_nits() for the aux.
> > >
> > > BugLink:
> > >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbu
> > >
> gzilla.opensuse.org%2Fshow_bug.cgi%3Fid%3D1180749&amp;data=04%7C01
> %7
> > >
> Calexander.deucher%40amd.com%7Ce5579cfe56f74b572f1508d8ca9ad0ac%7
> C3d
> > >
> d8961fe4884e608e11a82d994e183d%7C0%7C0%7C637482113562863043%7CU
> nknow
> > >
> n%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1ha
> Wwi
> > >
> LCJXVCI6Mn0%3D%7C1000&amp;sdata=HVtqM2r6oxSWd3XGGQZotO8wrvM
> qCTcwfq1L
> > > 2%2FeCmSE%3D&amp;reserved=0
> > > BugLink:
> > >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
> > > tlab.freedesktop.org%2Fdrm%2Famd%2F-
> %2Fissues%2F1438&amp;data=04%7C0
> > >
> 1%7Calexander.deucher%40amd.com%7Ce5579cfe56f74b572f1508d8ca9ad0
> ac%7
> > >
> C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637482113562863043%
> 7CUnk
> > >
> nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6I
> k1ha
> > >
> WwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=TdYgwNJ%2FvkuoDLNb9ATFb1P
> yznlp%2F
> > > P8TLuYSR%2BVkNqY%3D&amp;reserved=0
> > > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> >
> > This looks fine to me.  FWIW, I have a similar patch set here:
> > https://nam11.safelinks.protection.outlook.com/?url=https:%2F%2Fcgit.f
> >
> reedesktop.org%2F~agd5f%2Flinux%2Flog%2F%3Fh%3Dbacklight_wip&amp;
> data=
> >
> 04%7C01%7Calexander.deucher%40amd.com%7Ce5579cfe56f74b572f1508d8
> ca9ad0
> >
> ac%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637482113562863
> 043%7CU
> >
> nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI
> 6Ik1ha
> >
> WwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=aoMSY0nvHjrLocUPJtdgckqIH7x
> LUPbwpH0
> > ZjhuuJO8%3D&amp;reserved=0
> 
> I'm fine to scratch mine as long as the issue gets fixed :)
> 
> FWIW, the biggest problem so far was the aux channel backlight didn't work
> as expected, the actual backlight isn't changed by the backlight sysfs write.
> (And the sysfs read gives a bogus value, but it's not the cause of the non-
> working backlight control.)
> 
> Does the aux channel backlight really work with the current code?
> Or is this rather a device-specific issue (e.g. broken BIOS) and we might need
> to come up with a deny list or such?
> 

@Kazlauskas, Nicholas, @Siqueira, Rodrigo

Has there been any progress on the backlight fixes?

Alex
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

end of thread, other threads:[~2021-02-08 15:02 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-03 12:42 [PATCH 0/2] drm/amd/display: some backlight fixes Takashi Iwai
2021-02-03 12:42 ` Takashi Iwai
2021-02-03 12:42 ` [PATCH 1/2] drm/amd/display: Fix the brightness read via aux Takashi Iwai
2021-02-03 12:42   ` Takashi Iwai
2021-02-05 16:36   ` Alex Deucher
2021-02-05 16:36     ` Alex Deucher
2021-02-06 12:29     ` Takashi Iwai
2021-02-06 12:29       ` Takashi Iwai
2021-02-08 15:02       ` Deucher, Alexander
2021-02-08 15:02         ` Deucher, Alexander
2021-02-03 12:42 ` [PATCH 2/2] drm/amd/display: Add aux_backlight module option Takashi Iwai
2021-02-03 12:42   ` Takashi Iwai
2021-02-05 16:34   ` Alex Deucher
2021-02-05 16:34     ` Alex Deucher
2021-02-06 12:25     ` Takashi Iwai
2021-02-06 12:25       ` Takashi Iwai

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.