[PATCH 1/3] drm/amd/display: Add DC Debug mask to disable features for bringup

[Harry Wentland]

On 2020-05-12 12:54 p.m., Kazlauskas, Nicholas wrote:
> On 2020-05-12 12:45 p.m., Harry Wentland wrote:
>> [Why]
>> At bringup we want to be able to disable various power features.
>>
>> [How]
>> These features are already exposed as dc_debug_options and exercised
>> on other OSes. Create a new dc_debug_mask module parameter and expose
>> relevant bits, in particular
>>   * DC_DISABLE_PIPE_SPLIT
>>   * DC_DISABLE_STUTTER
>>   * DC_DISABLE_DSC
>>   * DC_DISABLE_CLOCK_GATING
>>
>> Signed-off-by: Harry Wentland <harry.wentland at amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h               |  1 +
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c           |  8 ++++++++
>>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 14 ++++++++++++++
>>   drivers/gpu/drm/amd/include/amd_shared.h          |  7 +++++++
>>   4 files changed, 30 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index 2a806cb55b78..13c8ccdb2948 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -173,6 +173,7 @@ extern int amdgpu_gpu_recovery;
>>   extern int amdgpu_emu_mode;
>>   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 struct amdgpu_mgpu_info mgpu_info;
>>   extern int amdgpu_ras_enable;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> index 589000938ab6..76707491657e 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> @@ -140,6 +140,7 @@ int amdgpu_emu_mode = 0;
>>   uint amdgpu_smu_memory_pool_size = 0;
>>   /* FBC (bit 0) disabled by default*/
>>   uint amdgpu_dc_feature_mask = 0;
>> +uint amdgpu_dc_debug_mask = 0;
>>   int amdgpu_async_gfx_ring = 1;
>>   int amdgpu_mcbp = 0;
>>   int amdgpu_discovery = -1;
>> @@ -714,6 +715,13 @@ MODULE_PARM_DESC(queue_preemption_timeout_ms,
>> "queue preemption timeout in ms (1
>>   MODULE_PARM_DESC(dcfeaturemask, "all stable DC features enabled
>> (default))");
>>   module_param_named(dcfeaturemask, amdgpu_dc_feature_mask, uint, 0444);
>>   +/**
>> + * DOC: dcdebugmask (uint)
>> + * Override display features enabled. See enum DC_DEBUG_MASK in
>> drivers/gpu/drm/amd/include/amd_shared.h.
>> + */
>> +MODULE_PARM_DESC(dcdebugmask, "all debug options disabled (default))");
>> +module_param_named(dcdebugmask, amdgpu_dc_debug_mask, uint, 0444);
>> +
>>   /**
>>    * DOC: abmlevel (uint)
>>    * Override the default ABM (Adaptive Backlight Management) level
>> used for DC
>> 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 60fe64aef11b..bf347ca43064 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -918,6 +918,20 @@ static int amdgpu_dm_init(struct amdgpu_device
>> *adev)
>>           goto error;
>>       }
>>   +    if (amdgpu_dc_debug_mask & DC_DISABLE_PIPE_SPLIT) {
>> +        adev->dm.dc->debug.force_single_disp_pipe_split = false;
>> +        adev->dm.dc->debug.pipe_split_policy = MPC_SPLIT_AVOID;
>> +    }
>> +
>> +    if (amdgpu_dc_debug_mask & DC_DISABLE_STUTTER)
>> +        adev->dm.dc->debug.disable_stutter = true;
>> +
>> +    if (amdgpu_dc_debug_mask & DC_DISABLE_DSC)
>> +        adev->dm.dc->debug.disable_dsc = true;
>> +
>> +    if (amdgpu_dc_debug_mask & DC_DISABLE_CLOCK_GATING)
>> +        adev->dm.dc->debug.disable_clock_gate = true;
>> +
> 
> Series is Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
> 
> Note that we fill in device defaults during dc_create, but there are
> some debug options which we unfortunately do reference during
> dc_create() itself which would apply some options too late in the sequence.
> 
> Fortunately, I don't think that's the case for any of these debug
> options, but it's something to keep in mind at least. That's what the
> init_flags were added for I think, and those generally have preference
> over debug option overrides when available (from my understanding).
> 

Thanks.

dc_debug isn't provided via dc_init_data, so we can only modify dc_debug
afterwards. if dc_create uses debug options that's a bit unfortunate but
we'd probably have to address that one-by-one if it matters.

Harry

> Regards,
> Nicholas Kazlauskas
> 
>>       r = dm_dmub_hw_init(adev);
>>       if (r) {
>>           DRM_ERROR("DMUB interface failed to initialize:
>> status=%d\n", r);
>> diff --git a/drivers/gpu/drm/amd/include/amd_shared.h
>> b/drivers/gpu/drm/amd/include/amd_shared.h
>> index d655a76bedc6..92126c54cb1c 100644
>> --- a/drivers/gpu/drm/amd/include/amd_shared.h
>> +++ b/drivers/gpu/drm/amd/include/amd_shared.h
>> @@ -150,6 +150,13 @@ enum DC_FEATURE_MASK {
>>       DC_PSR_MASK = 0x8,
>>   };
>>   +enum DC_DEBUG_MASK {
>> +    DC_DISABLE_PIPE_SPLIT = 0x1,
>> +    DC_DISABLE_STUTTER = 0x2,
>> +    DC_DISABLE_DSC = 0x4,
>> +    DC_DISABLE_CLOCK_GATING = 0x8
>> +};
>> +
>>   enum amd_dpm_forced_level;
>>   /**
>>    * struct amd_ip_funcs - general hooks for managing amdgpu IP Blocks
>>
>