RE: [Bug][Regression][Bisected] pp_table writes began to fail for Navi10 on amd-staging-drm-next
[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
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
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
[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
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
- 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
- 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
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 +-