Re: [PATCH 2/3] drm/i915: Change Mipi register definitions

[Daniel Vetter <daniel@ffwll.ch>]

On Sun, Jun 01, 2014 at 05:41:54AM +0000, Sharma, Shashank wrote:
> Hi Damien, 
> Please correct me if I am missing something, but the only reason we are
> seeing those extra formatting changes is, almost all of the old MIPI
> register definitions were beyond 80 char, and was checked in like that
> with warnings (How ?) So when I was checking for checkpatch errors, I
> saw that, and I tried to fix that in the previous patch. 

Checkpatch et al. isn't a hard rule here, and occasionally checkpatch is
just wrong. We have lots more exceptions in i915_reg.h since it makes it
easier to read in general.
-Daniel

> 
> Anyways, I will send another patch, as per your suggestions, which will include only the dev_priv->mmio_offset change, and rebase the other one on top of it.  
> 
> Regards
> Shashank
> -----Original Message-----
> From: Lespiau, Damien 
> Sent: Saturday, May 31, 2014 3:19 PM
> To: Sharma, Shashank
> Cc: Kumar, Shobhit; intel-gfx@lists.freedesktop.org; Vetter, Daniel; ville.syrjala@linux.intel.com
> Subject: Re: [PATCH 2/3] drm/i915: Change Mipi register definitions
> 
> On Sat, May 31, 2014 at 01:32:42PM +0530, Shashank Sharma wrote:
> > 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.
> > 
> > V4: Addressing review comments by Ville This patch removes all the 
> > un-necessary formatting changes.
> > V5: Addressing review comments by Damien Changed input variable name 
> > from tc to pipe
> > 
> > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> 
> I'm sorry if we haven't been clear enough, but in a patch that changes VLV_DISPLAY_BASE + 0xf00 to dev_priv->mipi_mmio_base + 0xf00, we can't
> have:
> 
> > -#define MIPI_PORT_CTRL(pipe)		_PIPE(pipe, _MIPIA_PORT_CTRL, _MIPIB_PORT_CTRL)
> > +#define MIPI_PORT_CTRL(pipe)			_PIPE(pipe, _MIPIA_PORT_CTRL, \
> > +						_MIPIB_PORT_CTRL)
> 
> That's the un-necessary formatting changes that Ville was talking about, and the "change only one thing per patch" I was talking about. In this case the change is "make VLV_DISPLAY_BASE + 0xfoo" to dev_priv->mipi_mmio_base + 0xf00", so the diff should only show that kind of changes.
> 
> Please bear with me for this one, let's get it "correct" and I'm sure the next ones will be easier.
> 
> --
> Damien
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch