Re: [PATCH 1/4] drm/i915/icl: Add VIDEO_DIP regsiters

From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: "Srivatsa, Anusha" <anusha.srivatsa@intel.com>
Cc: "Nikula, Jani" <jani.nikula@intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 1/4] drm/i915/icl: Add VIDEO_DIP regsiters
Date: Wed, 18 Jul 2018 17:51:23 -0700	[thread overview]
Message-ID: <20180719005122.GL2146@intel.com> (raw)
In-Reply-To: <20180718223405.GB2146@intel.com>

On Wed, Jul 18, 2018 at 03:34:05PM -0700, Rodrigo Vivi wrote:
> On Wed, Jul 18, 2018 at 03:06:45PM -0700, Srivatsa, Anusha wrote:
> > 
> > 
> > >-----Original Message-----
> > >From: Vivi, Rodrigo
> > >Sent: Wednesday, July 18, 2018 1:54 PM
> > >To: Navare, Manasi D <manasi.d.navare@intel.com>
> > >Cc: Srivatsa, Anusha <anusha.srivatsa@intel.com>; Nikula, Jani
> > ><jani.nikula@intel.com>; intel-gfx@lists.freedesktop.org
> > >Subject: Re: [Intel-gfx] [PATCH 1/4] drm/i915/icl: Add VIDEO_DIP regsiters
> > >
> > >On Wed, Jul 18, 2018 at 11:53:54AM -0700, Manasi Navare wrote:
> > >> On Tue, Jul 17, 2018 at 02:10:58PM -0700, Anusha Srivatsa wrote:
> > >> > From: "Srivatsa, Anusha" <anusha.srivatsa@intel.com>
> > >> >
> > >> > The Picture Parameter Set metadata for DSC has to be sent to the
> > >> > panel through secondary data packets. Add the error correction
> > >> > registers, data registers and control registers for the same.
> > >> >
> > >> > The control registers for  transcoders A and B are already defined
> > >> > and will be reused for Icelake purpose. This patch adds Control
> > >> > register for EDP and transcoder C apart from adding the PPS data and
> > >> > error registers.
> > >> >
> > >> > v2: reuse MMIO_TRANS2 for _PPS_DATA and _PPS_ECC.
> > >> > The  _MMIO_TRANS2(pipe, reg) macro definition takes care of the eDp
> > >> > case
> > >> >
> > >> > Cc: Jani Nikula <jani.nikula@intel.com>
> > >> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > >> > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > >> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > >>
> > >> Checked against the spec and tested it.
> > >>
> > >> Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
> > >>
> > >> > ---
> > >> >  drivers/gpu/drm/i915/i915_reg.h | 23 +++++++++++++++++++++++
> > >> >  1 file changed, 23 insertions(+)
> > >> >
> > >> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > >> > b/drivers/gpu/drm/i915/i915_reg.h index 1f222af..23e70a4 100644
> > >> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > >> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > >> > @@ -4605,6 +4605,16 @@ enum {
> > >> >  #define   VIDEO_DIP_ENABLE_GMP_HSW	(1 << 4)
> > >> >  #define   VIDEO_DIP_ENABLE_SPD_HSW	(1 << 0)
> > >> >
> > >> > +#define  DRM_DIP_ENABLE			(1 << 28)
> > >> > +#define  PSR_VSC_BIT_7_SET		(1 << 27)
> > >> > +#define  VSC_SELECT_MASK		(0x3 << 26)
> > >> > +#define  VSC_SELECT_SHIFT		26
> > >> > +#define  VSC_DIP_HW_HEA_DATA		(0 << 26)
> > >> > +#define  VSC_DIP_HW_HEA_SW_DATA		(1 << 26)
> > >> > +#define  VSC_DIP_HW_DATA_SW_HEA		(2 << 26)
> > >> > +#define  VSC_DIP_SW_HEA_DATA		(3 << 26)
> > >> > +#define  VDIP_ENABLE_PPS		(1 << 24)
> > >> > +
> > >> >  /* Panel power sequencing */
> > >> >  #define PPS_BASE			0x61200
> > >> >  #define VLV_PPS_BASE			(VLV_DISPLAY_BASE +
> > >PPS_BASE)
> > >> > @@ -7843,12 +7853,25 @@ enum {
> > >> >  #define _HSW_VIDEO_DIP_VSC_ECC_B	0x61344
> > >> >  #define _HSW_VIDEO_DIP_GCP_B		0x61210
> > >> >
> > >> > +/* Icelake PPS_DATA and _ECC DIP Registers.
> > >> > + * These are available for transcoders B,C and eDP.
> > >> > + * Adding the _A so as to reuse the _MMIO_TRANS2
> > >> > + * definition, with which it offsets to the right location.
> > >> > + */
> > >> > +
> > >> > +#define _ICL_VIDEO_DIP_PPS_DATA_A	0x60350
> > >> > +#define _ICL_VIDEO_DIP_PPS_DATA_B	0x61350
> > >> > +#define _ICL_VIDEO_DIP_PPS_ECC_A	0x603D4
> > >> > +#define _ICL_VIDEO_DIP_PPS_ECC_B	0x613D4
> > >> > +
> > >> >  #define HSW_TVIDEO_DIP_CTL(trans)		_MMIO_TRANS2(trans,
> > >_HSW_VIDEO_DIP_CTL_A)
> > >> >  #define HSW_TVIDEO_DIP_AVI_DATA(trans, i)	_MMIO_TRANS2(trans,
> > >_HSW_VIDEO_DIP_AVI_DATA_A + (i) * 4)
> > >> >  #define HSW_TVIDEO_DIP_VS_DATA(trans, i)	_MMIO_TRANS2(trans,
> > >_HSW_VIDEO_DIP_VS_DATA_A + (i) * 4)
> > >> >  #define HSW_TVIDEO_DIP_SPD_DATA(trans, i)	_MMIO_TRANS2(trans,
> > >_HSW_VIDEO_DIP_SPD_DATA_A + (i) * 4)
> > >> >  #define HSW_TVIDEO_DIP_GCP(trans)		_MMIO_TRANS2(trans,
> > >_HSW_VIDEO_DIP_GCP_A)
> > >> >  #define HSW_TVIDEO_DIP_VSC_DATA(trans, i)	_MMIO_TRANS2(trans,
> > >_HSW_VIDEO_DIP_VSC_DATA_A + (i) * 4)
> > >> > +#define ICL_VIDEO_DIP_PPS_DATA(trans, i)	_MMIO_TRANS2(trans,
> > >_ICL_VIDEO_DIP_PPS_DATA_A + (i) * 4)
> > >> > +#define ICL_VIDEO_DIP_PPS_ECC(trans, i)		_MMIO_TRANS2(trans,
> > >_ICL_VIDEO_DIP_PPS_ECC_A + (i) * 4)
> > >
> > >need to make checkpatch happy here ^
> > 
> > Rodrigo,
> >  There are actually some lines right above this that are over 80 characters.... 
> > I thought splitting the line might not look good here. :(
> 
> This is not what checkpatch complained about
> check your tabs vs spaces..

please accept my apologies. I got confused by my mutt here
and misread the checkpatch report.

pushed to dinq. Thanks for patches and reviews.

> 
> > 
> > 
> > Anusha  
> > >> >
> > >> >  #define _HSW_STEREO_3D_CTL_A		0x70020
> > >> >  #define   S3D_ENABLE			(1 << 31)
> > >> > --
> > >> > 2.7.4
> > >> >
> > >> _______________________________________________
> > >> Intel-gfx mailing list
> > >> Intel-gfx@lists.freedesktop.org
> > >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx