From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 2/2] drm/i915: Change Mipi register definitions Date: Wed, 21 May 2014 18:35:12 +0300 Message-ID: <20140521153512.GR27580@intel.com> References: <20140519161050.GB19476@strange.amr.corp.intel.com> <1400686019-29933-1-git-send-email-shashank.sharma@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id A40D46EA74 for ; Wed, 21 May 2014 08:35:30 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1400686019-29933-1-git-send-email-shashank.sharma@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Shashank Sharma Cc: jani.nikula@intel.com, daniel.vetter@intel.com, intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Wed, May 21, 2014 at 08:56:59PM +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. > = > V2: Addressing review comments by Damien, added follwing changes: > 1. Re-defined MIPI_DSI_FUNC_PRG using _PIPE macro, to remove > branching. > 2. Re-written _MIPIB_DSI_FUNC_PRG and _MIPIA_DSI_FUNC_PRG > in single line. > Signed-off-by: Shashank Sharma Sorry but this patch is a mess. Way too many pointless formatting changes in there. > --- > drivers/gpu/drm/i915/i915_reg.h | 689 +++++++++++++++++++++++----------= ------ > 1 file changed, 416 insertions(+), 273 deletions(-) > = > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_= reg.h > index c12a858..50d5e89 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -5655,20 +5655,23 @@ enum punit_power_well { > #define PIPE_CSC_POSTOFF_ME(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME, _P= IPE_B_CSC_POSTOFF_ME) > #define PIPE_CSC_POSTOFF_LO(pipe) _PIPE(pipe, _PIPE_A_CSC_POSTOFF_LO, _P= IPE_B_CSC_POSTOFF_LO) > = > -/* VLV MIPI registers */ > = > +/* =3D=3D=3D=3D MIPI registers =3D=3D=3D=3D */ This change is not needed. > + > +/* VLV port control */ > #define _MIPIA_PORT_CTRL (VLV_DISPLAY_BASE + 0x61190) > #define _MIPIB_PORT_CTRL (VLV_DISPLAY_BASE + 0x61700) > #define MIPI_PORT_CTRL(pipe) _PIPE(pipe, _MIPIA_PORT_CTRL, _MIPIB_PORT_= CTRL) Why isn't mipi_mmio_base used here? Does the register not need the new offset? I htink it would still be cleaner to use the mipi_mmio_offset for all the MIPI registers. > -#define DPI_ENABLE (1 << 31) /* A + B */ > + > +#define DPI_ENABLE (1 << 31) Not needed. > #define MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT 27 > #define MIPIA_MIPI4DPHY_DELAY_COUNT_MASK (0xf << 27) > #define DUAL_LINK_MODE_MASK (1 << 26) > #define DUAL_LINK_MODE_FRONT_BACK (0 << 26) > #define DUAL_LINK_MODE_PIXEL_ALTERNATIVE (1 << 26) > -#define DITHERING_ENABLE (1 << 25) /* A + B */ > +#define DITHERING_ENABLE (1 << 25) > #define FLOPPED_HSTX (1 << 23) > -#define DE_INVERT (1 << 19) /* XXX */ > +#define DE_INVERT (1 << 19) More unneeded comment changes. > #define MIPIA_FLISDSI_DELAY_COUNT_SHIFT 18 > #define MIPIA_FLISDSI_DELAY_COUNT_MASK (0xf << 18) > #define AFE_LATCHOUT (1 << 17) > @@ -5699,33 +5702,46 @@ enum punit_power_well { > #define LANE_CONFIGURATION_DUAL_LINK_A (1 << 0) > #define LANE_CONFIGURATION_DUAL_LINK_B (2 << 0) > = > +/* VLV tearing effect control */ > #define _MIPIA_TEARING_CTRL (VLV_DISPLAY_BASE + 0x61194) > #define _MIPIB_TEARING_CTRL (VLV_DISPLAY_BASE + 0x61704) > #define MIPI_TEARING_CTRL(pipe) _PIPE(pipe, _MIPIA_TEARING_CTRL, _MIPIB= _TEARING_CTRL) > -#define TEARING_EFFECT_DELAY_SHIFT 0 > -#define TEARING_EFFECT_DELAY_MASK (0xffff << 0) > +#define TEARING_EFFECT_DELAY_SHIFT 0 > +#define TEARING_EFFECT_DELAY_MASK (0xffff << 0) Bad formatting change. etc. Did you run this through some autoformatting tool or something? Please don't do that. A simple sed job should be all that's needed for mipi_mmio_offset. -- = Ville Syrj=E4l=E4 Intel OTC