Re: [PATCH v2] drm/i915: Clean up display pipe register accesses

["Antti Koskipää" <antti.koskipaa@linux.intel.com>]

On 01/24/14 14:52, Ville Syrjälä wrote:
> On Fri, Jan 24, 2014 at 02:13:14PM +0200, Antti Koskipaa wrote:
>> +#define PIPE_A_OFFSET	0x70000
>> +#define PIPE_B_OFFSET	0x71000
>> +#define PIPE_C_OFFSET	0x72000
> 
> I'd like a comment here to explain what PIPE_EDP_OFFSET
> actually means. Eg.:
> 
> /*
>  * There's actually no pipe EDP. Some pipe registers have
>  * simply shifted from the pipe to the transcoder, while
>  * keeping their original offset. Thus we need PIPE_EDP_OFFSET
>  * to access such registers in transcoder EDP.
>  */

Ok.

>>  #define TRANS_DDI_FUNC_CTL_B		0x61400
>>  #define TRANS_DDI_FUNC_CTL_C		0x62400
>>  #define TRANS_DDI_FUNC_CTL_EDP		0x6F400
>> -#define TRANS_DDI_FUNC_CTL(tran) _TRANSCODER(tran, TRANS_DDI_FUNC_CTL_A, \
>> -						   TRANS_DDI_FUNC_CTL_B)
>> +#define TRANS_DDI_FUNC_CTL(tran) (dev_priv->info->trans_ddi_func_offsets[tran])
>> +
> 
> Why is there a separate offset for these? They're all within the
> normal transcoder range, aren't they?

Yes, they are. Will use _TRANSCODER2 for those too.

>>  #define  TRANS_DDI_FUNC_ENABLE		(1<<31)
>>  /* Those bits are ignored by pipe EDP since it can only connect to DDI A */
>>  #define  TRANS_DDI_PORT_MASK		(7<<28)
>> @@ -5343,10 +5362,12 @@
>>  #define  TRANS_CLK_SEL_DISABLED		(0x0<<29)
>>  #define  TRANS_CLK_SEL_PORT(x)		((x+1)<<29)
>>  
>> -#define _TRANSA_MSA_MISC		0x60410
>> -#define _TRANSB_MSA_MISC		0x61410
>> -#define TRANS_MSA_MISC(tran) _TRANSCODER(tran, _TRANSA_MSA_MISC, \
>> -					       _TRANSB_MSA_MISC)
>> +#define TRANSA_MSA_MISC			0x60410
>> +#define TRANSB_MSA_MISC			0x61410
>> +#define TRANSC_MSA_MISC			0x62410
>> +#define TRANS_EDP_MSA_MISC		0x6f410
>> +#define TRANS_MSA_MISC(tran) (dev_priv->info->trans_msa_misc_offsets[tran])
> 
> ditto

Ditto.

-- 
- Antti