Re: [RFC PATCH v2 3/5] drm/amd/display: create DCN3-specific log for MPC state

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 2023-09-26 08:38, Melissa Wen wrote:
> On 09/25, Harry Wentland wrote:
>>
>>
>> On 2023-09-13 12:43, Melissa Wen wrote:
>>> Logging DCN3 MPC state was following DCN1 implementation that doesn't
>>> consider new DCN3 MPC color blocks. Create new elements according to
>>> DCN3 MPC color caps and a new DCN3-specific function for reading MPC
>>> data.
>>>
>>> Signed-off-by: Melissa Wen <mwen@xxxxxxxxxx>
>>> ---
>>>  .../gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c  | 55 ++++++++++++++++++-
>>>  drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h   | 13 +++++
>>>  2 files changed, 67 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c
>>> index d1500b223858..d164fbf89212 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c
>>> +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_mpc.c
>>> @@ -1382,8 +1382,61 @@ static void mpc3_set_mpc_mem_lp_mode(struct mpc *mpc)
>>>  	}
>>>  }
>>>  
>>> +static void mpc3_read_mpcc_state(
>>> +		struct mpc *mpc,
>>> +		int mpcc_inst,
>>> +		struct mpcc_state *s)
>>> +{
>>> +	struct dcn30_mpc *mpc30 = TO_DCN30_MPC(mpc);
>>> +	uint32_t rmu_status = 0xf;
>>> +
>>> +	REG_GET(MPCC_OPP_ID[mpcc_inst], MPCC_OPP_ID, &s->opp_id);
>>> +	REG_GET(MPCC_TOP_SEL[mpcc_inst], MPCC_TOP_SEL, &s->dpp_id);
>>> +	REG_GET(MPCC_BOT_SEL[mpcc_inst], MPCC_BOT_SEL, &s->bot_mpcc_id);
>>> +	REG_GET_4(MPCC_CONTROL[mpcc_inst], MPCC_MODE, &s->mode,
>>> +			MPCC_ALPHA_BLND_MODE, &s->alpha_mode,
>>> +			MPCC_ALPHA_MULTIPLIED_MODE, &s->pre_multiplied_alpha,
>>> +			MPCC_BLND_ACTIVE_OVERLAP_ONLY, &s->overlap_only);
>>> +	REG_GET_2(MPCC_STATUS[mpcc_inst], MPCC_IDLE, &s->idle,
>>> +			MPCC_BUSY, &s->busy);
>>> +
>>> +	/* Color blocks state */
>>> +	REG_GET(MPC_RMU_CONTROL, MPC_RMU0_MUX_STATUS, &rmu_status);
>>> +	if (rmu_status == mpcc_inst) {
>>> +		REG_GET(SHAPER_CONTROL[0],
>>> +			MPC_RMU_SHAPER_LUT_MODE_CURRENT, &s->shaper_lut_mode);
>>> +		REG_GET(RMU_3DLUT_MODE[0],
>>> +			MPC_RMU_3DLUT_MODE_CURRENT,  &s->lut3d_mode);
>>> +		REG_GET(RMU_3DLUT_READ_WRITE_CONTROL[0],
>>> +			MPC_RMU_3DLUT_30BIT_EN, &s->lut3d_bit_depth);
>>> +		REG_GET(RMU_3DLUT_MODE[0],
>>> +			MPC_RMU_3DLUT_SIZE, &s->lut3d_size);
>>> +	} else {
>>> +		REG_GET(SHAPER_CONTROL[1],
>>> +			MPC_RMU_SHAPER_LUT_MODE_CURRENT, &s->shaper_lut_mode);
>>> +		REG_GET(RMU_3DLUT_MODE[1],
>>> +			MPC_RMU_3DLUT_MODE_CURRENT,  &s->lut3d_mode);
>>> +		REG_GET(RMU_3DLUT_READ_WRITE_CONTROL[1],
>>> +			MPC_RMU_3DLUT_30BIT_EN, &s->lut3d_bit_depth);
>>> +		REG_GET(RMU_3DLUT_MODE[1],
>>> +			MPC_RMU_3DLUT_SIZE, &s->lut3d_size);
>>> +	}
>>> +         REG_GET_2(MPCC_OGAM_CONTROL[mpcc_inst],
>>> +		   MPCC_OGAM_MODE_CURRENT, &s->rgam_mode,
>>> +		   MPCC_OGAM_SELECT_CURRENT, &s->rgam_lut);
>>> +	REG_GET(MPCC_GAMUT_REMAP_MODE[mpcc_inst],
>>> +		MPCC_GAMUT_REMAP_MODE_CURRENT, &s->gamut_remap_mode);
>>> +	if (s->gamut_remap_mode == 1) {
>>> +		s->gamut_remap_c11_c12 = REG_READ(MPC_GAMUT_REMAP_C11_C12_A[mpcc_inst]);
>>> +		s->gamut_remap_c33_c34 = REG_READ(MPC_GAMUT_REMAP_C33_C34_A[mpcc_inst]);
>>> +	} else if (s->gamut_remap_mode == 2) {
>>> +		s->gamut_remap_c11_c12 = REG_READ(MPC_GAMUT_REMAP_C11_C12_B[mpcc_inst]);
>>> +		s->gamut_remap_c33_c34 = REG_READ(MPC_GAMUT_REMAP_C33_C34_B[mpcc_inst]);
>>
>> Any reason we're getting (and printing) only the first and last
>> coefficients? Is it to avoid printing lines that are too wide?
> 
> I'm unable to reach the other coefficients through this
> `MPC_GAMUT_REMAP` register prefix. But I'm probably missing something
> since I can see the registers using UMR. I'll try to find the right path
> and update it.
> 

>From dcn3_0_1_offset.h the registers are there:

> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C11_C12_A                                                         0x014c
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C11_C12_A_BASE_IDX                                                3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C13_C14_A                                                         0x014d
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C13_C14_A_BASE_IDX                                                3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C21_C22_A                                                         0x014e
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C21_C22_A_BASE_IDX                                                3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C23_C24_A                                                         0x014f
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C23_C24_A_BASE_IDX                                                3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C31_C32_A                                                         0x0150
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C31_C32_A_BASE_IDX                                                3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C33_C34_A                                                         0x0151
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C33_C34_A_BASE_IDX                                                3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C11_C12_B                                                         0x0152
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C11_C12_B_BASE_IDX                                                3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C13_C14_B                                                         0x0153
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C13_C14_B_BASE_IDX                                                3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C21_C22_B                                                         0x0154
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C21_C22_B_BASE_IDX                                                3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C23_C24_B                                                         0x0155
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C23_C24_B_BASE_IDX                                                3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C31_C32_B                                                         0x0156
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C31_C32_B_BASE_IDX                                                3
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C33_C34_B                                                         0x0157
> #define mmMPCC_OGAM0_MPC_GAMUT_REMAP_C33_C34_B_BASE_IDX                                                3

But in dcn30_mpc.h we only define the first and last:

> #define MPC_REG_LIST_DCN3_0(inst)\
...
> 	SRII(MPC_GAMUT_REMAP_C11_C12_A, MPCC_OGAM, inst),\
> 	SRII(MPC_GAMUT_REMAP_C33_C34_A, MPCC_OGAM, inst),\
> 	SRII(MPC_GAMUT_REMAP_C11_C12_B, MPCC_OGAM, inst),\
> 	SRII(MPC_GAMUT_REMAP_C33_C34_B, MPCC_OGAM, inst),\

If you add the othes it should work.

The reason for the MPC_REG_LIST_DCN3_0 (and others like it) is to
(a) abstract the _offset.h definitions away and give us common code
    for register access for all generations, no matter if the offsets
    change, and
(b) to give us compile errors if a register definition is missing.

Harry


> Melissa
> 
>>
>> Harry
>>
>>> +	}
>>> +}
>>> +
>>>  static const struct mpc_funcs dcn30_mpc_funcs = {
>>> -	.read_mpcc_state = mpc1_read_mpcc_state,
>>> +	.read_mpcc_state = mpc3_read_mpcc_state,
>>>  	.insert_plane = mpc1_insert_plane,
>>>  	.remove_mpcc = mpc1_remove_mpcc,
>>>  	.mpc_init = mpc1_mpc_init,
>>> diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h b/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h
>>> index 8d86159d9de0..e60b3503605b 100644
>>> --- a/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h
>>> +++ b/drivers/gpu/drm/amd/display/dc/inc/hw/mpc.h
>>> @@ -193,6 +193,19 @@ struct mpcc_state {
>>>  	uint32_t overlap_only;
>>>  	uint32_t idle;
>>>  	uint32_t busy;
>>> +	uint32_t shaper_lut_mode;
>>> +	uint32_t lut3d_mode;
>>> +	uint32_t lut3d_bit_depth;
>>> +	uint32_t lut3d_size;
>>> +	uint32_t rgam_mode;
>>> +	uint32_t rgam_lut;
>>> +	uint32_t gamut_remap_mode;
>>> +	uint32_t gamut_remap_c11_c12;
>>> +	uint32_t gamut_remap_c13_c14;
>>> +	uint32_t gamut_remap_c21_c22;
>>> +	uint32_t gamut_remap_c23_c24;
>>> +	uint32_t gamut_remap_c31_c32;
>>> +	uint32_t gamut_remap_c33_c34;
>>>  };
>>>  
>>>  /**
>>




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux