[Intel-gfx] [PATCH 3/3] drm/i915: Use transcoder as index to MIPI regs

Damien Lespiau damien.lespiau at intel.com
Mon Jun 2 15:26:53 CEST 2014


On Mon, Jun 02, 2014 at 01:55:13PM +0100, Sharma, Shashank wrote:
> Hi Damien, 
> 
> Can you please point out these, as this patch is re-based on latest
> 2/3, I was expecting this to be without any inconsistency. 
> I personally checked for any <80 char formatting, which is not
> required. But if I missed any, I can again fix this, please let me
> know. 

At this point, there's no "rule". As Daniel said earlier the 80 chars
limit is a soft one, esp. in headers declaring list of registers.

For the inconsistencies, it's just a personal preference, I would try to
make all defines look alike, right now you have:

#define MIPI_DPI_CONTROL(tc)		_TRANSCODER(tc, _MIPIA_DPI_CONTROL, \
					_MIPIB_DPI_CONTROL)


#define MIPI_GEN_FIFO_STAT(tc)	_TRANSCODER(tc, _MIPIA_GEN_FIFO_STAT, \
					_MIPIB_GEN_FIFO_STAT)


#define MIPI_READ_DATA_VALID(tc)	_TRANSCODER(tc, \
				_MIPIA_READ_DATA_VALID, _MIPIB_READ_DATA_VALID)


All different alignments. Not something I would ever do, but there's no rule
against it per se, hence the r-b.

You have a couple more of debatable splits:

#define _MIPIA_CLK_LANE_SWITCH_TIME_CNT		(dev_priv->mipi_mmio_base \
							+ 0xb088)

#define MIPI_READ_DATA_RETURN(tc, n) \
	(_TRANSCODER(tc, _MIPIA_READ_DATA_RETURN0, _MIPIB_READ_DATA_RETURN0) \
					+ 4 * (n)) /* n: 0...7 */

Esp. for the first one, these are cases where the "< 80 chars" split goes
against readibility.

Someone may ask you to fix those "bad" splits, not me this time though.

-- 
Damien



More information about the Intel-gfx mailing list