RE: [Bug][Regression][Bisected] pp_table writes began to fail for Navi10 on amd-staging-drm-next

2020-08-02 Thread Quan, Evan
[AMD Official Use Only - Internal Distribution Only]

Hi Matt,

I just sent out two patches to fix the issue reported here. It worked on my 
local env.
Please confirm on your platform.
https://lists.freedesktop.org/archives/amd-gfx/2020-August/052245.html
https://lists.freedesktop.org/archives/amd-gfx/2020-August/052246.html

BR,
Evan
-Original Message-
From: Quan, Evan
Sent: Friday, July 31, 2020 9:20 AM
To: Matt Coffin ; amd-gfx@lists.freedesktop.org
Cc: Alex Deucher 
Subject: RE: [Bug][Regression][Bisected] pp_table writes began to fail for 
Navi10 on amd-staging-drm-next

Thanks for reporting this. I will check it.

BR
Evan
-Original Message-
From: Matt Coffin 
Sent: Thursday, July 30, 2020 10:25 PM
To: Quan, Evan ; amd-gfx@lists.freedesktop.org
Cc: Alex Deucher 
Subject: [Bug][Regression][Bisected] pp_table writes began to fail for Navi10 
on amd-staging-drm-next

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

Hey Evan,

I've been having an issue with uploading `pp_table`s on recent 
`amd-staging-drm-next` kernels. I bisected the issue, and it came back to a 
commit of yours - ec8ee23f610578c71885a36ddfcf58d35cccab67.

I didn't have your gitlab handle to CC you on the issue, so I thought I'd at 
least alert you to it.

Here's a link to the issue on GitLab:
https://gitlab.freedesktop.org/drm/amd/-/issues/1243

I'd appreciate any help or insight you could offer here as I work on a fix.

First bad commit header:

commit ec8ee23f610578c71885a36ddfcf58d35cccab67 (refs/bisect/bad)
Author: Evan Quan 
Date:   Wed Jun 10 16:52:32 2020 +0800

drm/amd/powerplay: update Navi10 default dpm table setup

Cache all clocks levels for every dpm table. They are needed
by other APIs.

Change-Id: I8114cf31e6ec8c9af4578d51749eb213befdcc71
Signed-off-by: Evan Quan 
Reviewed-by: Alex Deucher 

Thanks everyone, and cheers,
Matt Coffin
-BEGIN PGP SIGNATURE-

iQIzBAEBCAAdFiEEwz6NkrTNYgbdBT2N4mzKau7FyAcFAl8i2FAACgkQ4mzKau7F
yAfhzQ//a4EgvriD6AdSFUgYmmdMnf1iN+cIDj8JnTuoOgRs+hV2ObIywyE4HZCC
yd1+GdY8/P1VerdC6d5wpTozUBWQGvCnJRnF2kJ7ZKyjsITtpOh6qUWTPbKi7GrU
qz9DD+BIMIcRW/qp+sqVBE0z6vEJSbEkoCF4njV/Qzy178XFNnt0rrScGTsIa+yj
gvUw51m3T9qzMl9pglpUxZv9k3jQfirMZYDv2O2Ds7CIptPGeHDiCW7aNRAPYrmF
UYnt7a2W6fhOrG7x2In8sMECsE/uNa8GMqxAXenRPDok093owHs9zg5lVLbHvvA8
z3iB11n8VT6TknW6YpCm5uE3Lynq1acAIoVO/8m7z56SAIco+xxTkOKt5SWxZGvS
wQN9teaMXYSzHR/d/RTSZXjxBcOYgokIJrcb1hmJd5zw/gvZeeMfhCpyAbnhbOpR
wC6wbic39xNza9sIZrg5NCSx6UcBpfwjKygN7HSutua1nZi9WbvviY6LL69JYhmK
Jyg8RgjgiUPrLomtZx60vLi54dAupyDqJZv2FRHMQTxC+7HagHcDItK8a1KIoNIW
wsvUbonsyyMMxvrTbMPcrTm+yRkFmaP+1fnpgjL1WCHgd4imcl68sV22D64OCT/4
YzfWzhN+dIIUnfy0eWiiLMlOsEWGBN+YPVdLxodK0okjDcQJQTA=
=CzXz
-END PGP SIGNATURE-
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH 1/2] drm/amd/powerplay: update swSMU VCN/JPEG PG logics

2020-08-02 Thread Evan Quan
Add lock protections and avoid unnecessary actions
if the PG state is already the same as required.

Change-Id: I01400b84151d3ac6e3c8b0d7e264f9a68a9c2092
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c| 57 ++-
 drivers/gpu/drm/amd/powerplay/arcturus_ppt.c  |  4 --
 .../gpu/drm/amd/powerplay/inc/amdgpu_smu.h|  6 +-
 drivers/gpu/drm/amd/powerplay/navi10_ppt.c|  8 ---
 drivers/gpu/drm/amd/powerplay/renoir_ppt.c|  8 ---
 .../drm/amd/powerplay/sienna_cichlid_ppt.c|  9 ---
 drivers/gpu/drm/amd/powerplay/smu_internal.h  |  2 -
 7 files changed, 60 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c 
b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index cf9c5205ef08..85b04c48bd09 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -133,6 +133,56 @@ int smu_get_dpm_freq_range(struct smu_context *smu,
return ret;
 }
 
+static int smu_dpm_set_vcn_enable(struct smu_context *smu,
+ bool enable)
+{
+   struct smu_power_context *smu_power = >smu_power;
+   struct smu_power_gate *power_gate = _power->power_gate;
+   int ret = 0;
+
+   if (!smu->ppt_funcs->dpm_set_vcn_enable)
+   return 0;
+
+   mutex_lock(_gate->vcn_gate_lock);
+
+   if (atomic_read(_gate->vcn_gated) ^ enable)
+   goto out;
+
+   ret = smu->ppt_funcs->dpm_set_vcn_enable(smu, enable);
+   if (!ret)
+   atomic_set(_gate->vcn_gated, !enable);
+
+out:
+   mutex_unlock(_gate->vcn_gate_lock);
+
+   return ret;
+}
+
+static int smu_dpm_set_jpeg_enable(struct smu_context *smu,
+  bool enable)
+{
+   struct smu_power_context *smu_power = >smu_power;
+   struct smu_power_gate *power_gate = _power->power_gate;
+   int ret = 0;
+
+   if (!smu->ppt_funcs->dpm_set_jpeg_enable)
+   return 0;
+
+   mutex_lock(_gate->jpeg_gate_lock);
+
+   if (atomic_read(_gate->jpeg_gated) ^ enable)
+   goto out;
+
+   ret = smu->ppt_funcs->dpm_set_jpeg_enable(smu, enable);
+   if (!ret)
+   atomic_set(_gate->jpeg_gated, !enable);
+
+out:
+   mutex_unlock(_gate->jpeg_gate_lock);
+
+   return ret;
+}
+
 /**
  * smu_dpm_set_power_gate - power gate/ungate the specific IP block
  *
@@ -644,6 +694,11 @@ static int smu_sw_init(void *handle)
smu->power_profile_mode = PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT;
smu->default_power_profile_mode = PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT;
 
+   atomic_set(>smu_power.power_gate.vcn_gated, 1);
+   atomic_set(>smu_power.power_gate.jpeg_gated, 1);
+   mutex_init(>smu_power.power_gate.vcn_gate_lock);
+   mutex_init(>smu_power.power_gate.jpeg_gate_lock);
+
smu->workload_mask = 1 << 
smu->workload_prority[PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT];
smu->workload_prority[PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT] = 0;
smu->workload_prority[PP_SMC_POWER_PROFILE_FULLSCREEN3D] = 1;
@@ -1974,7 +2029,7 @@ int smu_read_sensor(struct smu_context *smu,
*size = 4;
break;
case AMDGPU_PP_SENSOR_VCN_POWER_STATE:
-   *(uint32_t *)data = smu->smu_power.power_gate.vcn_gated ? 0 : 1;
+   *(uint32_t *)data = 
atomic_read(>smu_power.power_gate.vcn_gated) ? 0: 1;
*size = 4;
break;
case AMDGPU_PP_SENSOR_MIN_FAN_RPM:
diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c 
b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
index d678534ddc69..a2ba6633fc21 100644
--- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
@@ -1894,8 +1894,6 @@ static bool arcturus_is_dpm_running(struct smu_context 
*smu)
 
 static int arcturus_dpm_set_vcn_enable(struct smu_context *smu, bool enable)
 {
-   struct smu_power_context *smu_power = >smu_power;
-   struct smu_power_gate *power_gate = _power->power_gate;
int ret = 0;
 
if (enable) {
@@ -1906,7 +1904,6 @@ static int arcturus_dpm_set_vcn_enable(struct smu_context 
*smu, bool enable)
return ret;
}
}
-   power_gate->vcn_gated = false;
} else {
if (smu_cmn_feature_is_enabled(smu, SMU_FEATURE_VCN_PG_BIT)) {
ret = smu_cmn_feature_set_enabled(smu, 
SMU_FEATURE_VCN_PG_BIT, 0);
@@ -1915,7 +1912,6 @@ static int arcturus_dpm_set_vcn_enable(struct smu_context 
*smu, bool enable)
return ret;
}
}
-   power_gate->vcn_gated = true;
}
 
return ret;
diff --git a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h 
b/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
index ec2d2aa7f4ec..23c2279bd500 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/amdgpu_smu.h
+++ 

[PATCH 2/2] drm/amd/powerplay: put VCN/JPEG into PG ungate state before dpm table setup

2020-08-02 Thread Evan Quan
As VCN related dpm table setup needs VCN be in PG ungate state. Same logics
applies to JPEG.

Change-Id: I94335efc4e0424cfe0991e984c938998fd8f1287
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 38 +-
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c 
b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index 85b04c48bd09..1349d05c5f3d 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -134,7 +134,8 @@ int smu_get_dpm_freq_range(struct smu_context *smu,
 }
 
 static int smu_dpm_set_vcn_enable(struct smu_context *smu,
- bool enable)
+ bool enable,
+ int *previous_pg_state)
 {
struct smu_power_context *smu_power = >smu_power;
struct smu_power_gate *power_gate = _power->power_gate;
@@ -148,6 +149,9 @@ static int smu_dpm_set_vcn_enable(struct smu_context *smu,
if (atomic_read(_gate->vcn_gated) ^ enable)
goto out;
 
+   if (previous_pg_state)
+   *previous_pg_state = atomic_read(_gate->vcn_gated);
+
ret = smu->ppt_funcs->dpm_set_vcn_enable(smu, enable);
if (!ret)
atomic_set(_gate->vcn_gated, !enable);
@@ -159,7 +163,8 @@ static int smu_dpm_set_vcn_enable(struct smu_context *smu,
 }
 
 static int smu_dpm_set_jpeg_enable(struct smu_context *smu,
-  bool enable)
+  bool enable,
+  int *previous_pg_state)
 {
struct smu_power_context *smu_power = >smu_power;
struct smu_power_gate *power_gate = _power->power_gate;
@@ -173,6 +178,9 @@ static int smu_dpm_set_jpeg_enable(struct smu_context *smu,
if (atomic_read(_gate->jpeg_gated) ^ enable)
goto out;
 
+   if (previous_pg_state)
+   *previous_pg_state = atomic_read(_gate->jpeg_gated);
+
ret = smu->ppt_funcs->dpm_set_jpeg_enable(smu, enable);
if (!ret)
atomic_set(_gate->jpeg_gated, !enable);
@@ -212,7 +220,7 @@ int smu_dpm_set_power_gate(struct smu_context *smu, 
uint32_t block_type,
 */
case AMD_IP_BLOCK_TYPE_UVD:
case AMD_IP_BLOCK_TYPE_VCN:
-   ret = smu_dpm_set_vcn_enable(smu, !gate);
+   ret = smu_dpm_set_vcn_enable(smu, !gate, NULL);
if (ret)
dev_err(smu->adev->dev, "Failed to power %s VCN!\n",
gate ? "gate" : "ungate");
@@ -230,7 +238,7 @@ int smu_dpm_set_power_gate(struct smu_context *smu, 
uint32_t block_type,
gate ? "gate" : "ungate");
break;
case AMD_IP_BLOCK_TYPE_JPEG:
-   ret = smu_dpm_set_jpeg_enable(smu, !gate);
+   ret = smu_dpm_set_jpeg_enable(smu, !gate, NULL);
if (ret)
dev_err(smu->adev->dev, "Failed to power %s JPEG!\n",
gate ? "gate" : "ungate");
@@ -407,6 +415,7 @@ static int smu_late_init(void *handle)
 {
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
struct smu_context *smu = >smu;
+   int vcn_gate, jpeg_gate;
int ret = 0;
 
if (!smu->pm_enabled)
@@ -418,6 +427,14 @@ static int smu_late_init(void *handle)
return ret;
}
 
+   /*
+* 1. Power up VCN/JPEG as the succeeding smu_set_default_dpm_table()
+*needs VCN/JPEG up.
+* 2. Save original gate states and then we can restore back afterwards.
+*/
+   smu_dpm_set_vcn_enable(smu, true, _gate);
+   smu_dpm_set_jpeg_enable(smu, true, _gate);
+
/*
 * Set initialized values (get from vbios) to dpm tables context such as
 * gfxclk, memclk, dcefclk, and etc. And enable the DPM feature for each
@@ -429,6 +446,11 @@ static int smu_late_init(void *handle)
return ret;
}
 
+   /* Restore back to original VCN/JPEG power gate states */
+   smu_dpm_set_vcn_enable(smu, !vcn_gate, NULL);
+   smu_dpm_set_jpeg_enable(smu, !vcn_gate, NULL);
+
+
ret = smu_populate_umd_state_clk(smu);
if (ret) {
dev_err(adev->dev, "Failed to populate UMD state clocks!\n");
@@ -991,8 +1013,8 @@ static int smu_hw_init(void *handle)
 
if (smu->is_apu) {
smu_powergate_sdma(>smu, false);
-   smu_dpm_set_vcn_enable(smu, true);
-   smu_dpm_set_jpeg_enable(smu, true);
+   smu_dpm_set_vcn_enable(smu, true, NULL);
+   smu_dpm_set_jpeg_enable(smu, true, NULL);
smu_set_gfx_cgpg(>smu, true);
}
 
@@ -1132,8 +1154,8 @@ static int smu_hw_fini(void *handle)
 
if (smu->is_apu) {
smu_powergate_sdma(>smu, true);
-   smu_dpm_set_vcn_enable(smu, 

RE: [PATCH 1/2] drm/amdgpu: fix reload KMD hang on KIQ

2020-08-02 Thread Liu, Monk
[AMD Official Use Only - Internal Distribution Only]

>>In gfx_v10_0_sw_fini the KIQ ring gets freed. Wouldn't that be the right 
>>place to stop the KIQ

KIQ (CPC) will never being stopped (the DISABLE on CPC is skipped for SRIOV ) 
for SRIOV in SW_FINI because SRIOV relies on KIQ to do world switch

But this is really a weird bug because even with the same approach it doesn't 
make KIQ (CP) hang on GFX9, only GFX10 need this patch 

By now I do not have other good explanation or better fix than this one

_
Monk Liu|GPU Virtualization Team |AMD


-Original Message-
From: Kuehling, Felix 
Sent: Friday, July 31, 2020 9:57 PM
To: Liu, Monk ; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/2] drm/amdgpu: fix reload KMD hang on KIQ

In gfx_v10_0_sw_fini the KIQ ring gets freed. Wouldn't that be the right place 
to stop the KIQ? Otherwise KIQ will hang as soon as someone allocates the 
memory that was previously used for the KIQ ring buffer and overwrites it with 
something that's not a valid PM4 packet.

Regards,
  Felix

Am 2020-07-31 um 3:51 a.m. schrieb Monk Liu:
> KIQ will hang if we try below steps:
> modprobe amdgpu
> rmmod amdgpu
> modprobe amdgpu sched_hw_submission=4
>
> the cause is that due to KIQ is always living there even after we
> unload KMD thus when doing the realod of KMD KIQ will crash upon its
> register programed with different values with the previous
> configuration (the config like HQD addr, ring size, is easily changed
> if we alter the sched_hw_submission)
>
> the fix is we must inactive KIQ first before touching any of its
> registgers
>
> Signed-off-by: Monk Liu 
> ---
>  drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> index db9f1e8..f571e25 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> @@ -6433,6 +6433,9 @@ static int gfx_v10_0_kiq_init_register(struct 
> amdgpu_ring *ring)
>  struct v10_compute_mqd *mqd = ring->mqd_ptr;
>  int j;
>
> +/* activate the queue */
> +WREG32_SOC15(GC, 0, mmCP_HQD_ACTIVE, 0);
> +
>  /* disable wptr polling */
>  WREG32_FIELD15(GC, 0, CP_PQ_WPTR_POLL_CNTL, EN, 0);
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Enabling AMDGPU by default for SI & CIK

2020-08-02 Thread Bas Nieuwenhuizen
Hi all,

Now that we have recently made some progress on getting feature parity
with the Radeon driver for SI, I'm wondering what it would take to
make AMDGPU the default driver for these generations.

As far as I understand AMDGPU has had these features for CIK for a
while already but it is still not the default driver. What would it
take to make it the default? What is missing and/or broken?

- Bas

(I did some statistics on the Steam Survey, and both SI and CIK have
around 9% marketshare for Linux+ AMD there:
https://gitlab.freedesktop.org/snippets/1128 Would be cool if we could
enable Vulkan and VR out of the box on these cards)
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH v1 19/22] drm/amdgpu/atom: Backlight update

2020-08-02 Thread Sam Ravnborg
- Use macros for initialization
- Replace direct access to backlight_properties with get and set
  operations

Signed-off-by: Sam Ravnborg 
Cc: Alex Deucher 
Cc: Christian König 
Cc: amd-gfx@lists.freedesktop.org
Cc: Sam Ravnborg 
---
 drivers/gpu/drm/amd/amdgpu/atombios_encoders.c | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c 
b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
index 1e94a9b652f7..4338577eb7ba 100644
--- a/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
+++ b/drivers/gpu/drm/amd/amdgpu/atombios_encoders.c
@@ -122,15 +122,16 @@ amdgpu_atombios_encoder_set_backlight_level(struct 
amdgpu_encoder *amdgpu_encode
 
 static u8 amdgpu_atombios_encoder_backlight_level(struct backlight_device *bd)
 {
+   int brightness = backlight_get_brightness(bd);
u8 level;
 
/* Convert brightness to hardware level */
-   if (bd->props.brightness < 0)
+   if (brightness < 0)
level = 0;
-   else if (bd->props.brightness > AMDGPU_MAX_BL_LEVEL)
+   else if (brightness > AMDGPU_MAX_BL_LEVEL)
level = AMDGPU_MAX_BL_LEVEL;
else
-   level = bd->props.brightness;
+   level = brightness;
 
return level;
 }
@@ -165,6 +166,7 @@ static const struct backlight_ops 
amdgpu_atombios_encoder_backlight_ops = {
 void amdgpu_atombios_encoder_init_backlight(struct amdgpu_encoder 
*amdgpu_encoder,
 struct drm_connector *drm_connector)
 {
+   DECLARE_BACKLIGHT_INIT_RAW(props, 0, AMDGPU_MAX_BL_LEVEL);
struct drm_device *dev = amdgpu_encoder->base.dev;
struct amdgpu_device *adev = dev->dev_private;
struct backlight_device *bd;
@@ -193,9 +195,6 @@ void amdgpu_atombios_encoder_init_backlight(struct 
amdgpu_encoder *amdgpu_encode
goto error;
}
 
-   memset(, 0, sizeof(props));
-   props.max_brightness = AMDGPU_MAX_BL_LEVEL;
-   props.type = BACKLIGHT_RAW;
snprintf(bl_name, sizeof(bl_name),
 "amdgpu_bl%d", dev->primary->index);
bd = backlight_device_register(bl_name, drm_connector->kdev,
@@ -212,8 +211,8 @@ void amdgpu_atombios_encoder_init_backlight(struct 
amdgpu_encoder *amdgpu_encode
dig = amdgpu_encoder->enc_priv;
dig->bl_dev = bd;
 
-   bd->props.brightness = 
amdgpu_atombios_encoder_get_backlight_brightness(bd);
-   bd->props.power = FB_BLANK_UNBLANK;
+   backlight_set_brightness(bd, 
amdgpu_atombios_encoder_get_backlight_brightness(bd));
+   backlight_set_power_on(bd);
backlight_update_status(bd);
 
DRM_INFO("amdgpu atom DIG backlight initialized\n");
-- 
2.25.1

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


[PATCH v1 18/22] drm/radeon: Backlight update

2020-08-02 Thread Sam Ravnborg
- Use macros for initialization
- Replace direct access to backlight_properties with get and set
  operations

Signed-off-by: Sam Ravnborg 
Cc: Alex Deucher 
Cc: Christian König 
Cc: amd-gfx@lists.freedesktop.org
---
 drivers/gpu/drm/radeon/atombios_encoders.c| 23 ++-
 .../gpu/drm/radeon/radeon_legacy_encoders.c   | 15 ++--
 2 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/radeon/atombios_encoders.c 
b/drivers/gpu/drm/radeon/atombios_encoders.c
index cc5ee1b3af84..c9431af12fed 100644
--- a/drivers/gpu/drm/radeon/atombios_encoders.c
+++ b/drivers/gpu/drm/radeon/atombios_encoders.c
@@ -145,14 +145,15 @@ atombios_set_backlight_level(struct radeon_encoder 
*radeon_encoder, u8 level)
 static u8 radeon_atom_bl_level(struct backlight_device *bd)
 {
u8 level;
+   int brightness = backlight_get_brightness(bd);
 
/* Convert brightness to hardware level */
-   if (bd->props.brightness < 0)
+   if (brightness < 0)
level = 0;
-   else if (bd->props.brightness > RADEON_MAX_BL_LEVEL)
+   else if (brightness > RADEON_MAX_BL_LEVEL)
level = RADEON_MAX_BL_LEVEL;
else
-   level = bd->props.brightness;
+   level = brightness;
 
return level;
 }
@@ -185,12 +186,13 @@ static const struct backlight_ops 
radeon_atom_backlight_ops = {
 void radeon_atom_backlight_init(struct radeon_encoder *radeon_encoder,
struct drm_connector *drm_connector)
 {
+   DECLARE_BACKLIGHT_INIT_RAW(props, 0, RADEON_MAX_BL_LEVEL);
struct drm_device *dev = radeon_encoder->base.dev;
struct radeon_device *rdev = dev->dev_private;
struct backlight_device *bd;
-   struct backlight_properties props;
struct radeon_backlight_privdata *pdata;
struct radeon_encoder_atom_dig *dig;
+   int brightness;
char bl_name[16];
 
/* Mac laptops with multiple GPUs use the gmux driver for backlight
@@ -215,9 +217,6 @@ void radeon_atom_backlight_init(struct radeon_encoder 
*radeon_encoder,
goto error;
}
 
-   memset(, 0, sizeof(props));
-   props.max_brightness = RADEON_MAX_BL_LEVEL;
-   props.type = BACKLIGHT_RAW;
snprintf(bl_name, sizeof(bl_name),
 "radeon_bl%d", dev->primary->index);
bd = backlight_device_register(bl_name, drm_connector->kdev,
@@ -232,15 +231,17 @@ void radeon_atom_backlight_init(struct radeon_encoder 
*radeon_encoder,
dig = radeon_encoder->enc_priv;
dig->bl_dev = bd;
 
-   bd->props.brightness = radeon_atom_backlight_get_brightness(bd);
+   brightness = radeon_atom_backlight_get_brightness(bd);
/* Set a reasonable default here if the level is 0 otherwise
 * fbdev will attempt to turn the backlight on after console
 * unblanking and it will try and restore 0 which turns the backlight
 * off again.
 */
-   if (bd->props.brightness == 0)
-   bd->props.brightness = RADEON_MAX_BL_LEVEL;
-   bd->props.power = FB_BLANK_UNBLANK;
+
+   if (brightness == 0)
+   brightness = RADEON_MAX_BL_LEVEL;
+   backlight_set_brightness(bd, brightness);
+   backlight_set_power_on(bd);
backlight_update_status(bd);
 
DRM_INFO("radeon atom DIG backlight initialized\n");
diff --git a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c 
b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
index 44d060f75318..cf2d1776b975 100644
--- a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
+++ b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
@@ -323,14 +323,15 @@ static uint8_t radeon_legacy_lvds_level(struct 
backlight_device *bd)
 {
struct radeon_backlight_privdata *pdata = bl_get_data(bd);
uint8_t level;
+   int brightness = backlight_get_brightness(bd);
 
/* Convert brightness to hardware level */
-   if (bd->props.brightness < 0)
+   if (brightness < 0)
level = 0;
-   else if (bd->props.brightness > RADEON_MAX_BL_LEVEL)
+   else if (brightness > RADEON_MAX_BL_LEVEL)
level = RADEON_MAX_BL_LEVEL;
else
-   level = bd->props.brightness;
+   level = brightness;
 
if (pdata->negative)
level = RADEON_MAX_BL_LEVEL - level;
@@ -371,6 +372,7 @@ static const struct backlight_ops radeon_backlight_ops = {
 void radeon_legacy_backlight_init(struct radeon_encoder *radeon_encoder,
  struct drm_connector *drm_connector)
 {
+   DECLARE_BACKLIGHT_INIT_RAW(props, 0, RADEON_MAX_BL_LEVEL);
struct drm_device *dev = radeon_encoder->base.dev;
struct radeon_device *rdev = dev->dev_private;
struct backlight_device *bd;
@@ -394,9 +396,6 @@ void radeon_legacy_backlight_init(struct radeon_encoder 
*radeon_encoder,
goto error;
}
 
-   memset(, 0, 

[RFC PATCH v1 0/22] backlight: add init macros and accessors

2020-08-02 Thread Sam Ravnborg
The backlight drivers uses several different patterns when registering
a backlight:

- Register backlight and assign properties later
- Define a local backlight_properties variable and use memset
- Define a const backlight_properties and assign relevant properties

On top of this there was differences in what members was assigned in
backlight_properties.

To align how backlight drivers are initialized introduce following helper 
macros:
- DECLARE_BACKLIGHT_INIT_FIRMWARE()
- DECLARE_BACKLIGHT_INIT_PLATFORM()
- DECLARE_BACKLIGHT_INIT_RAW()

The macros are introduced in patch 2.

The backlight drivers used direct access to backlight_properties.
Encapsulate these in get/set access operations resulting in following benefits:
- The drivers no longer need to be concerned about the confusing power states,
  as there is now only a set_power_on() and set_power_off() operation.
- The access methods can be called with a NULL pointer so logic around the
  access can be made simpler.
- The code is in most cases more readable with the access operations.
- When everyone uses the access methods refactroring in the backlight core is 
simpler.

The get/set operations are introduced in patch 3.

The first patch trims backlight_update_status() so it can be called with a NULL
backlight_device. Then the called do not need to add this check just to avoid
a NULL reference.

The fourth patch introduce the new macros and get/set operations for the
gpio backlight driver, as an example.

The remaining patches updates most backlight users in drivers/gpu/drm/*
With this patch set applied then:
- Almost all references to FB_BLANK* are gone from drm/*
- All panel drivers uses devm_ variant for registering backlight
- Almost all direct references to backlight properties are gone

The drm/* patches are  used as examples how drivers can benefit from the
new macros and get/set operations.

Individual patches are only sent to the people listed in the patch + a few more.
Please check https://lore.kernel.org/dri-devel/ for the full series.

Feedback welcome!

Sam

Cc: Alex Deucher 
Cc: amd-gfx@lists.freedesktop.org
Cc: Andrzej Hajda 
Cc: Christian König 
Cc: Chris Wilson 
Cc: Daniel Thompson 
Cc: Ezequiel Garcia 
Cc: Hans de Goede 
Cc: Hoegeun Kwon 
Cc: Inki Dae 
Cc: Jani Nikula 
Cc: Jernej Skrabec 
Cc: Jingoo Han 
Cc: Jonas Karlman 
Cc: Joonas Lahtinen 
Cc: Jyri Sarha 
Cc: Kieran Bingham 
Cc: Konrad Dybcio 
Cc: Laurent Pinchart 
Cc: Lee Jones 
Cc: Linus Walleij 
Cc: linux-renesas-...@vger.kernel.org
Cc: Maarten Lankhorst 
Cc: Manasi Navare 
Cc: Neil Armstrong 
Cc: Patrik Jakobsson 
Cc: Paweł Chmiel 
Cc: Philippe CORNU 
Cc: Rob Clark 
Cc: Robert Chiras 
Cc: Rodrigo Vivi 
Cc: Sam Ravnborg 
Cc: Sebastian Reichel 
Cc: Thierry Reding 
Cc: Tomi Valkeinen 
Cc: "Ville Syrjälä" 
Cc: Vinay Simha BN 
Cc: Wambui Karuga 
Cc: Zheng Bin 

Sam Ravnborg (22):
  backlight: Silently fail backlight_update_status() if no device
  backlight: Add DECLARE_* macro for device registration
  backlight: Add get/set operations for brightness/power properties
  backlight: gpio: Use DECLARE_BACKLIGHT_INIT_RAW and get/setters
  drm/gma500: Backlight support
  drm/panel: asus-z00t-tm5p5-n35596: Backlight update
  drm/panel: jdi-lt070me05000: Backlight update
  drm/panel: novatek-nt35510: Backlight update
  drm/panel: orisetech-otm8009a: Backlight update
  drm/panel: raydium-rm67191: Backlight update
  drm/panel: samsung-s6e63m0: Backlight update
  drm/panel: samsung-s6e63j0x03: Backlight update
  drm/panel: samsung-s6e3ha2: Backlight update
  drm/panel: sony-acx424akp: Backlight update
  drm/panel: sony-acx565akm: Backlight update
  drm/bridge: parade-ps8622: Backlight update
  drm/tilcdc: Backlight update
  drm/radeon: Backlight update
  drm/amdgpu/atom: Backlight update
  drm/i915: Backlight update
  drm/omap: display: Backlight update
  drm/shmobile: Backlight update

 drivers/gpu/drm/amd/amdgpu/atombios_encoders.c |  15 ++-
 drivers/gpu/drm/bridge/parade-ps8622.c |  43 
 drivers/gpu/drm/gma500/backlight.c |  35 ++
 drivers/gpu/drm/gma500/cdv_device.c|  29 +++--
 drivers/gpu/drm/gma500/mdfld_device.c  |   9 +-
 drivers/gpu/drm/gma500/oaktrail_device.c   |  10 +-
 drivers/gpu/drm/gma500/opregion.c  |   2 +-
 drivers/gpu/drm/gma500/psb_device.c|  10 +-
 drivers/gpu/drm/gma500/psb_drv.c   |   8 +-
 drivers/gpu/drm/i915/display/intel_panel.c |  88 +++
 drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c|  35 ++
 .../gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c   |  15 +--
 drivers/gpu/drm/panel/panel-jdi-lt070me05000.c |  17 ++-
 drivers/gpu/drm/panel/panel-novatek-nt35510.c  |   9 +-
 drivers/gpu/drm/panel/panel-orisetech-otm8009a.c   |  14 +--
 drivers/gpu/drm/panel/panel-raydium-rm67191.c  |  11 +-