On 2022-11-01 11:33, Filip Moc wrote: > Hello Harry, > > thank you for your response. > >> amdgpu.backlight_min=2:-1 > > almost :-) > > Array elements in module parameters are separated by commas not colons. > So for cmdline it should look like this: > amdgpu.backlight_min=2,-1 > > Though you can just drop the ,-1 relying on kernel leaving the rest of array > untouched. Which I would recommend as there is no point for user to > follow AMDGPU_DM_MAX_NUM_EDP. > Only when you need to override some other display than display 0 then you may > need to use -1. E.g. backlight_min=-1,2 overrides display 1 to min backlight=2 > while keeping display 0 with no override. > > When amdgpu is loaded as a kernel module, backlight_min can also be passed as a > parameter to modprobe, e.g.: > modprobe backlight_min=2 > So in that case you probably want to add something like > options amdgpu backlight_min=2 to /etc/modprobe.d/amdgpu.conf > (and also run update-initramfs if amdgpu is loaded by initramfs). > > I'll add some examples to commit message in v2. > Awesome. Thanks. Harry > Filip > > > V Mon, Oct 31, 2022 at 10:24:25AM -0400, Harry Wentland napsal(a): >> On 2022-10-29 15:13, Filip Moc wrote: >>> There are some devices on which amdgpu won't allow user to set brightness >>> to sufficiently low values even though the hardware would support it just >>> fine. >>> >>> This usually happens in two cases when either configuration of brightness >>> levels via ACPI/ATIF is not available and amdgpu falls back to defaults >>> (currently 12 for minimum level) which may be too high for some devices or >>> even the configuration via ATIF is available but the minimum brightness >>> level provided by the manufacturer is set to unreasonably high value. >>> >>> In either case user can use this new module parameter to adjust the >>> minimum allowed backlight brightness level. >>> >> >> Thanks for this patch and covering all the bases. >> >> It might be useful to have an example in the commit description on >> how to set the array property. I assume it looks like this if I >> wanted to set the first device to a minimum of 2 and leave the default >> for the 2nd one: >> >> amdgpu.backlight_min=2:-1 >> >> Either way, this patch is >> Reviewed-by: Harry Wentland <harry.wentland@xxxxxxx> >> >> Harry >> >>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=203439 >>> Signed-off-by: Filip Moc <dev@xxxxxxx> >>> --- >>> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 +++ >>> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 15 +++++++++++++++ >>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 15 +++++++++++++++ >>> 3 files changed, 33 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h >>> index 0e6ddf05c23c..c5445402c49d 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h >>> @@ -200,6 +200,9 @@ extern uint amdgpu_dc_debug_mask; >>> extern uint amdgpu_dc_visual_confirm; >>> extern uint amdgpu_dm_abm_level; >>> extern int amdgpu_backlight; >>> +#ifdef CONFIG_DRM_AMD_DC >>> +extern int amdgpu_backlight_override_min[]; >>> +#endif >>> 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 16f6a313335e..f2fb549ac52f 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c >>> @@ -43,6 +43,7 @@ >>> #include "amdgpu_irq.h" >>> #include "amdgpu_dma_buf.h" >>> #include "amdgpu_sched.h" >>> +#include "amdgpu_dm.h" >>> #include "amdgpu_fdinfo.h" >>> #include "amdgpu_amdkfd.h" >>> >>> @@ -853,6 +854,20 @@ int amdgpu_backlight = -1; >>> MODULE_PARM_DESC(backlight, "Backlight control (0 = pwm, 1 = aux, -1 auto (default))"); >>> module_param_named(backlight, amdgpu_backlight, bint, 0444); >>> >>> +/** >>> + * DOC: backlight_min (array of int) >>> + * Override minimum allowed backlight brightness signal (per display). >>> + * Must be less than the maximum brightness signal. >>> + * Negative value means no override. >>> + * >>> + * Defaults to all -1 (no override on any display). >>> + */ >>> +#ifdef CONFIG_DRM_AMD_DC >>> +int amdgpu_backlight_override_min[AMDGPU_DM_MAX_NUM_EDP] = {[0 ... (AMDGPU_DM_MAX_NUM_EDP-1)] = -1}; >>> +MODULE_PARM_DESC(backlight_min, "Override minimum backlight brightness signal (0..max-1, -1 = no override (default))"); >>> +module_param_array_named(backlight_min, amdgpu_backlight_override_min, int, NULL, 0444); >>> +#endif >>> + >>> /** >>> * 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 eb4ce7216104..e2c36ba93d05 100644 >>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >>> @@ -3911,6 +3911,21 @@ static void amdgpu_dm_update_backlight_caps(struct amdgpu_display_manager *dm, >>> dm->backlight_caps[bl_idx].min_input_signal = AMDGPU_DM_DEFAULT_MIN_BACKLIGHT; >>> dm->backlight_caps[bl_idx].max_input_signal = AMDGPU_DM_DEFAULT_MAX_BACKLIGHT; >>> #endif >>> + >>> + if (amdgpu_backlight_override_min[bl_idx] >= 0) { >>> + if (amdgpu_backlight_override_min[bl_idx] < dm->backlight_caps[bl_idx].max_input_signal) { >>> + DRM_INFO("amdgpu: backlight[%i]: overriding minimum brightness from %i to %i\n", >>> + bl_idx, >>> + dm->backlight_caps[bl_idx].min_input_signal, >>> + amdgpu_backlight_override_min[bl_idx]); >>> + dm->backlight_caps[bl_idx].min_input_signal = amdgpu_backlight_override_min[bl_idx]; >>> + } else { >>> + DRM_ERROR("amdgpu: backlight[%i]: minimum brightness override (%i) is not below maximum (%i)\n", >>> + bl_idx, >>> + amdgpu_backlight_override_min[bl_idx], >>> + dm->backlight_caps[bl_idx].max_input_signal); >>> + } >>> + } >>> } >>> >>> static int get_brightness_range(const struct amdgpu_dm_backlight_caps *caps, >>> >>> base-commit: d8c03bfe146fd5e081a252cd34f3f12ca0255357 >>
