[Intel-gfx] [PATCH 2/2] drm/i915: Change Mipi register definitions

[Damien Lespiau]

On Fri, May 30, 2014 at 09:05:41AM +0100, Sharma, Shashank wrote:
> From: Sharma, Shashank 
> Sent: Thursday, May 22, 2014 5:02 PM
> To: intel-gfx at lists.freedesktop.org; Lespiau, Damien; ville.syrjala at linux.intel.com; Vetter, Daniel
> Cc: Kumar, Shobhit; Sharma, Shashank
> Subject: [PATCH 2/2] drm/i915: Change Mipi register definitions
> 
> Re-define MIPI register definitions in such a way that most of the existing DSI code can be re-used for future platforms. Register definitions are re-written using MMIO offset variable, so that without changing the existing sequence, same code can be generically applied.
> 
> V3: Addressing review comments by Damien and Ville, added follwing changes:
> 1. Replaced _PIPE with _TRANSCODER call, no branching added.
> 2. Removed all the un-necessary formatting changes from previous patch.

Ooops, I noticed that "oh, he's doing two things in the same patch" but
forgot to actually send any mail. So here it is.

While the patch looks correct, we try really hard to follow "1 patch, 1
change" esp. when an explanation is needed for each change. I see two
changes here:

  1/ the mipi_mmio_base change
  2/ the _PIPE->_TRANSCODER change

This commit should only contain 1/. I would do separate commit for 2,
with an explanation. Something like:

  drm/i915: Use the MIPI transcoder to index MIPI registers

  Conceptually, the MIPI registers are addressed by the MIPI transcoder
  index, not the pipe. It doesn't matter right now, because there's a
  1:1 relationship between pipes and MIPI transcoders, but that change
  allows us to break that link in the future.

Could you also not reindent the _TRANSCODER() to < 80 chars, I think in
that case it removes a bit of readability.

-- 
Damien