Re: [Intel-gfx] [PATCH 4/4] drm/i915: Extend skl+ crc sources with more planes

2019-02-20 Thread Ville Syrjälä
On Thu, Feb 14, 2019 at 06:07:05PM -0800, Dhinakaran Pandiyan wrote:
> On Thu, 2019-02-14 at 21:22 +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä 
> > 
> > On skl the crc registers were extended to provide plane crcs
> > for up to 7 planes. Add the new crc sources.
> > 
> > The current code uses the ivb+ register definitions for skl+
> > which does happen to work as the plane1, plane2, and dmux/pf
> > bits happen the match what ivb+ had. So no bug in the current
> > code.
> > 
> > Signed-off-by: Ville Syrjälä 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h   |  5 ++
> >  drivers/gpu/drm/i915/i915_reg.h   |  9 
> >  drivers/gpu/drm/i915/intel_pipe_crc.c | 76
> > ++-
> >  3 files changed, 88 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 4e11d970cbcf..8607c1e9ed02 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1196,6 +1196,11 @@ enum intel_pipe_crc_source {
> > INTEL_PIPE_CRC_SOURCE_NONE,
> > INTEL_PIPE_CRC_SOURCE_PLANE1,
> > INTEL_PIPE_CRC_SOURCE_PLANE2,
> > +   INTEL_PIPE_CRC_SOURCE_PLANE3,
> > +   INTEL_PIPE_CRC_SOURCE_PLANE4,
> > +   INTEL_PIPE_CRC_SOURCE_PLANE5,
> > +   INTEL_PIPE_CRC_SOURCE_PLANE6,
> > +   INTEL_PIPE_CRC_SOURCE_PLANE7,
> > INTEL_PIPE_CRC_SOURCE_PIPE,
> > /* TV/DP on pre-gen5/vlv can't use the pipe source. */
> > INTEL_PIPE_CRC_SOURCE_TV,
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 0df8c6e76da7..5286536e9cb8 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4017,6 +4017,15 @@ enum {
> >  /* Pipe A CRC regs */
> >  #define _PIPE_CRC_CTL_A0x60050
> >  #define   PIPE_CRC_ENABLE  (1 << 31)
> > +/* skl+ source selection */
> > +#define   PIPE_CRC_SOURCE_PLANE_1_SKL  (0 << 28)
> > +#define   PIPE_CRC_SOURCE_PLANE_2_SKL  (2 << 28)
> > +#define   PIPE_CRC_SOURCE_DMUX_SKL (4 << 28)
> > +#define   PIPE_CRC_SOURCE_PLANE_3_SKL  (6 << 28)
> > +#define   PIPE_CRC_SOURCE_PLANE_4_SKL  (7 << 28)
> > +#define   PIPE_CRC_SOURCE_PLANE_5_SKL  (5 << 28)
> > +#define   PIPE_CRC_SOURCE_PLANE_6_SKL  (3 << 28)
> > +#define   PIPE_CRC_SOURCE_PLANE_7_SKL  (1 << 28)
> >  /* ivb+ source selection */
> >  #define   PIPE_CRC_SOURCE_PRIMARY_IVB  (0 << 29)
> >  #define   PIPE_CRC_SOURCE_SPRITE_IVB   (1 << 29)
> > diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c
> > b/drivers/gpu/drm/i915/intel_pipe_crc.c
> > index 66bb7b031537..e521f82ba5d9 100644
> > --- a/drivers/gpu/drm/i915/intel_pipe_crc.c
> > +++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
> > @@ -34,6 +34,11 @@ static const char * const pipe_crc_sources[] = {
> > [INTEL_PIPE_CRC_SOURCE_NONE] = "none",
> > [INTEL_PIPE_CRC_SOURCE_PLANE1] = "plane1",
> > [INTEL_PIPE_CRC_SOURCE_PLANE2] = "plane2",
> > +   [INTEL_PIPE_CRC_SOURCE_PLANE3] = "plane3",
> > +   [INTEL_PIPE_CRC_SOURCE_PLANE4] = "plane4",
> > +   [INTEL_PIPE_CRC_SOURCE_PLANE5] = "plane5",
> > +   [INTEL_PIPE_CRC_SOURCE_PLANE6] = "plane6",
> > +   [INTEL_PIPE_CRC_SOURCE_PLANE7] = "plane7",
> > [INTEL_PIPE_CRC_SOURCE_PIPE] = "pipe",
> > [INTEL_PIPE_CRC_SOURCE_TV] = "TV",
> > [INTEL_PIPE_CRC_SOURCE_DP_B] = "DP-B",
> > @@ -368,6 +373,50 @@ static int ivb_pipe_crc_ctl_reg(struct
> > drm_i915_private *dev_priv,
> > return 0;
> >  }
> >  
> > +static int skl_pipe_crc_ctl_reg(struct drm_i915_private *dev_priv,
> > +   enum pipe pipe,
> > +   enum intel_pipe_crc_source *source,
> > +   uint32_t *val,
> > +   bool set_wa)
> 
> set_wa is unused. 

Dropped. And pushed.

Thanks for the reviews.

-- 
Ville Syrjälä
Intel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/4] drm/i915: Extend skl+ crc sources with more planes

2019-02-15 Thread Dhinakaran Pandiyan
On Thu, 2019-02-14 at 21:22 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> On skl the crc registers were extended to provide plane crcs
> for up to 7 planes. Add the new crc sources.
> 
> The current code uses the ivb+ register definitions for skl+
> which does happen to work as the plane1, plane2, and dmux/pf
> bits happen the match what ivb+ had. So no bug in the current
> code.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/i915_drv.h   |  5 ++
>  drivers/gpu/drm/i915/i915_reg.h   |  9 
>  drivers/gpu/drm/i915/intel_pipe_crc.c | 76
> ++-
>  3 files changed, 88 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 4e11d970cbcf..8607c1e9ed02 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1196,6 +1196,11 @@ enum intel_pipe_crc_source {
>   INTEL_PIPE_CRC_SOURCE_NONE,
>   INTEL_PIPE_CRC_SOURCE_PLANE1,
>   INTEL_PIPE_CRC_SOURCE_PLANE2,
> + INTEL_PIPE_CRC_SOURCE_PLANE3,
> + INTEL_PIPE_CRC_SOURCE_PLANE4,
> + INTEL_PIPE_CRC_SOURCE_PLANE5,
> + INTEL_PIPE_CRC_SOURCE_PLANE6,
> + INTEL_PIPE_CRC_SOURCE_PLANE7,
>   INTEL_PIPE_CRC_SOURCE_PIPE,
>   /* TV/DP on pre-gen5/vlv can't use the pipe source. */
>   INTEL_PIPE_CRC_SOURCE_TV,
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index 0df8c6e76da7..5286536e9cb8 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4017,6 +4017,15 @@ enum {
>  /* Pipe A CRC regs */
>  #define _PIPE_CRC_CTL_A  0x60050
>  #define   PIPE_CRC_ENABLE(1 << 31)
> +/* skl+ source selection */
> +#define   PIPE_CRC_SOURCE_PLANE_1_SKL(0 << 28)
> +#define   PIPE_CRC_SOURCE_PLANE_2_SKL(2 << 28)
> +#define   PIPE_CRC_SOURCE_DMUX_SKL   (4 << 28)
> +#define   PIPE_CRC_SOURCE_PLANE_3_SKL(6 << 28)
> +#define   PIPE_CRC_SOURCE_PLANE_4_SKL(7 << 28)
> +#define   PIPE_CRC_SOURCE_PLANE_5_SKL(5 << 28)
> +#define   PIPE_CRC_SOURCE_PLANE_6_SKL(3 << 28)
> +#define   PIPE_CRC_SOURCE_PLANE_7_SKL(1 << 28)
>  /* ivb+ source selection */
>  #define   PIPE_CRC_SOURCE_PRIMARY_IVB(0 << 29)
>  #define   PIPE_CRC_SOURCE_SPRITE_IVB (1 << 29)
> diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c
> b/drivers/gpu/drm/i915/intel_pipe_crc.c
> index 66bb7b031537..e521f82ba5d9 100644
> --- a/drivers/gpu/drm/i915/intel_pipe_crc.c
> +++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
> @@ -34,6 +34,11 @@ static const char * const pipe_crc_sources[] = {
>   [INTEL_PIPE_CRC_SOURCE_NONE] = "none",
>   [INTEL_PIPE_CRC_SOURCE_PLANE1] = "plane1",
>   [INTEL_PIPE_CRC_SOURCE_PLANE2] = "plane2",
> + [INTEL_PIPE_CRC_SOURCE_PLANE3] = "plane3",
> + [INTEL_PIPE_CRC_SOURCE_PLANE4] = "plane4",
> + [INTEL_PIPE_CRC_SOURCE_PLANE5] = "plane5",
> + [INTEL_PIPE_CRC_SOURCE_PLANE6] = "plane6",
> + [INTEL_PIPE_CRC_SOURCE_PLANE7] = "plane7",
>   [INTEL_PIPE_CRC_SOURCE_PIPE] = "pipe",
>   [INTEL_PIPE_CRC_SOURCE_TV] = "TV",
>   [INTEL_PIPE_CRC_SOURCE_DP_B] = "DP-B",
> @@ -368,6 +373,50 @@ static int ivb_pipe_crc_ctl_reg(struct
> drm_i915_private *dev_priv,
>   return 0;
>  }
>  
> +static int skl_pipe_crc_ctl_reg(struct drm_i915_private *dev_priv,
> + enum pipe pipe,
> + enum intel_pipe_crc_source *source,
> + uint32_t *val,
> + bool set_wa)

set_wa is unused. 




___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/4] drm/i915: Extend skl+ crc sources with more planes

2019-02-14 Thread Rodrigo Vivi
On Thu, Feb 14, 2019 at 11:29:08PM +0200, Ville Syrjälä wrote:
> On Thu, Feb 14, 2019 at 12:47:23PM -0800, Rodrigo Vivi wrote:
> > On Thu, Feb 14, 2019 at 09:22:19PM +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä 
> > > 
> > > On skl the crc registers were extended to provide plane crcs
> > > for up to 7 planes. Add the new crc sources.
> > > 
> > > The current code uses the ivb+ register definitions for skl+
> > > which does happen to work as the plane1, plane2, and dmux/pf
> > > bits happen the match what ivb+ had. So no bug in the current
> > > code.
> > > 
> > > Signed-off-by: Ville Syrjälä 
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.h   |  5 ++
> > >  drivers/gpu/drm/i915/i915_reg.h   |  9 
> > >  drivers/gpu/drm/i915/intel_pipe_crc.c | 76 ++-
> > >  3 files changed, 88 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h 
> > > b/drivers/gpu/drm/i915/i915_drv.h
> > > index 4e11d970cbcf..8607c1e9ed02 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -1196,6 +1196,11 @@ enum intel_pipe_crc_source {
> > >   INTEL_PIPE_CRC_SOURCE_NONE,
> > >   INTEL_PIPE_CRC_SOURCE_PLANE1,
> > >   INTEL_PIPE_CRC_SOURCE_PLANE2,
> > > + INTEL_PIPE_CRC_SOURCE_PLANE3,
> > > + INTEL_PIPE_CRC_SOURCE_PLANE4,
> > > + INTEL_PIPE_CRC_SOURCE_PLANE5,
> > > + INTEL_PIPE_CRC_SOURCE_PLANE6,
> > > + INTEL_PIPE_CRC_SOURCE_PLANE7,
> > >   INTEL_PIPE_CRC_SOURCE_PIPE,
> > >   /* TV/DP on pre-gen5/vlv can't use the pipe source. */
> > >   INTEL_PIPE_CRC_SOURCE_TV,
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h 
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index 0df8c6e76da7..5286536e9cb8 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -4017,6 +4017,15 @@ enum {
> > >  /* Pipe A CRC regs */
> > >  #define _PIPE_CRC_CTL_A  0x60050
> > >  #define   PIPE_CRC_ENABLE(1 << 31)
> > > +/* skl+ source selection */
> > > +#define   PIPE_CRC_SOURCE_PLANE_1_SKL(0 << 28)
> > > +#define   PIPE_CRC_SOURCE_PLANE_2_SKL(2 << 28)
> > > +#define   PIPE_CRC_SOURCE_DMUX_SKL   (4 << 28)
> > > +#define   PIPE_CRC_SOURCE_PLANE_3_SKL(6 << 28)
> > > +#define   PIPE_CRC_SOURCE_PLANE_4_SKL(7 << 28)
> > > +#define   PIPE_CRC_SOURCE_PLANE_5_SKL(5 << 28)
> > > +#define   PIPE_CRC_SOURCE_PLANE_6_SKL(3 << 28)
> > > +#define   PIPE_CRC_SOURCE_PLANE_7_SKL(1 << 28)
> > 
> > I got myself staring at spec for a while trying to
> > understand the logic of this sequence...
> 
> Did you find any logic in it? I honestly can't remember anymore why I
> did it like this.

no no... I'm not criticizing you. Your way is the most
clean and organized one.

My surprise came from the fact that I couldn't find
any logic on how those bits got selected at first place. ;)

> 
> > 
> > 
> > Reviewed-by: Rodrigo Vivi 
> > 
> > 
> > 
> > >  /* ivb+ source selection */
> > >  #define   PIPE_CRC_SOURCE_PRIMARY_IVB(0 << 29)
> > >  #define   PIPE_CRC_SOURCE_SPRITE_IVB (1 << 29)
> > > diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c 
> > > b/drivers/gpu/drm/i915/intel_pipe_crc.c
> > > index 66bb7b031537..e521f82ba5d9 100644
> > > --- a/drivers/gpu/drm/i915/intel_pipe_crc.c
> > > +++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
> > > @@ -34,6 +34,11 @@ static const char * const pipe_crc_sources[] = {
> > >   [INTEL_PIPE_CRC_SOURCE_NONE] = "none",
> > >   [INTEL_PIPE_CRC_SOURCE_PLANE1] = "plane1",
> > >   [INTEL_PIPE_CRC_SOURCE_PLANE2] = "plane2",
> > > + [INTEL_PIPE_CRC_SOURCE_PLANE3] = "plane3",
> > > + [INTEL_PIPE_CRC_SOURCE_PLANE4] = "plane4",
> > > + [INTEL_PIPE_CRC_SOURCE_PLANE5] = "plane5",
> > > + [INTEL_PIPE_CRC_SOURCE_PLANE6] = "plane6",
> > > + [INTEL_PIPE_CRC_SOURCE_PLANE7] = "plane7",
> > >   [INTEL_PIPE_CRC_SOURCE_PIPE] = "pipe",
> > >   [INTEL_PIPE_CRC_SOURCE_TV] = "TV",
> > >   [INTEL_PIPE_CRC_SOURCE_DP_B] = "DP-B",
> > > @@ -368,6 +373,50 @@ static int ivb_pipe_crc_ctl_reg(struct 
> > > drm_i915_private *dev_priv,
> > >   return 0;
> > >  }
> > >  
> > > +static int skl_pipe_crc_ctl_reg(struct drm_i915_private *dev_priv,
> > > + enum pipe pipe,
> > > + enum intel_pipe_crc_source *source,
> > > + uint32_t *val,
> > > + bool set_wa)
> > > +{
> > > + if (*source == INTEL_PIPE_CRC_SOURCE_AUTO)
> > > + *source = INTEL_PIPE_CRC_SOURCE_PIPE;
> > > +
> > > + switch (*source) {
> > > + case INTEL_PIPE_CRC_SOURCE_PLANE1:
> > > + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PLANE_1_SKL;
> > > + break;
> > > + case INTEL_PIPE_CRC_SOURCE_PLANE2:
> > > + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PLANE_2_SKL;
> > > + break;
> > > + case INTEL_PIPE_CRC_SOURCE_PLANE3:
> > > + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PLANE_3_SKL;
> > > + break;
> > > + case INTEL_PIPE_CRC_SOURCE_PLANE4:
> > > + *v

Re: [Intel-gfx] [PATCH 4/4] drm/i915: Extend skl+ crc sources with more planes

2019-02-14 Thread Ville Syrjälä
On Thu, Feb 14, 2019 at 12:47:23PM -0800, Rodrigo Vivi wrote:
> On Thu, Feb 14, 2019 at 09:22:19PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä 
> > 
> > On skl the crc registers were extended to provide plane crcs
> > for up to 7 planes. Add the new crc sources.
> > 
> > The current code uses the ivb+ register definitions for skl+
> > which does happen to work as the plane1, plane2, and dmux/pf
> > bits happen the match what ivb+ had. So no bug in the current
> > code.
> > 
> > Signed-off-by: Ville Syrjälä 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h   |  5 ++
> >  drivers/gpu/drm/i915/i915_reg.h   |  9 
> >  drivers/gpu/drm/i915/intel_pipe_crc.c | 76 ++-
> >  3 files changed, 88 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h 
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 4e11d970cbcf..8607c1e9ed02 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1196,6 +1196,11 @@ enum intel_pipe_crc_source {
> > INTEL_PIPE_CRC_SOURCE_NONE,
> > INTEL_PIPE_CRC_SOURCE_PLANE1,
> > INTEL_PIPE_CRC_SOURCE_PLANE2,
> > +   INTEL_PIPE_CRC_SOURCE_PLANE3,
> > +   INTEL_PIPE_CRC_SOURCE_PLANE4,
> > +   INTEL_PIPE_CRC_SOURCE_PLANE5,
> > +   INTEL_PIPE_CRC_SOURCE_PLANE6,
> > +   INTEL_PIPE_CRC_SOURCE_PLANE7,
> > INTEL_PIPE_CRC_SOURCE_PIPE,
> > /* TV/DP on pre-gen5/vlv can't use the pipe source. */
> > INTEL_PIPE_CRC_SOURCE_TV,
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h 
> > b/drivers/gpu/drm/i915/i915_reg.h
> > index 0df8c6e76da7..5286536e9cb8 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4017,6 +4017,15 @@ enum {
> >  /* Pipe A CRC regs */
> >  #define _PIPE_CRC_CTL_A0x60050
> >  #define   PIPE_CRC_ENABLE  (1 << 31)
> > +/* skl+ source selection */
> > +#define   PIPE_CRC_SOURCE_PLANE_1_SKL  (0 << 28)
> > +#define   PIPE_CRC_SOURCE_PLANE_2_SKL  (2 << 28)
> > +#define   PIPE_CRC_SOURCE_DMUX_SKL (4 << 28)
> > +#define   PIPE_CRC_SOURCE_PLANE_3_SKL  (6 << 28)
> > +#define   PIPE_CRC_SOURCE_PLANE_4_SKL  (7 << 28)
> > +#define   PIPE_CRC_SOURCE_PLANE_5_SKL  (5 << 28)
> > +#define   PIPE_CRC_SOURCE_PLANE_6_SKL  (3 << 28)
> > +#define   PIPE_CRC_SOURCE_PLANE_7_SKL  (1 << 28)
> 
> I got myself staring at spec for a while trying to
> understand the logic of this sequence...

Did you find any logic in it? I honestly can't remember anymore why I
did it like this.

> 
> 
> Reviewed-by: Rodrigo Vivi 
> 
> 
> 
> >  /* ivb+ source selection */
> >  #define   PIPE_CRC_SOURCE_PRIMARY_IVB  (0 << 29)
> >  #define   PIPE_CRC_SOURCE_SPRITE_IVB   (1 << 29)
> > diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c 
> > b/drivers/gpu/drm/i915/intel_pipe_crc.c
> > index 66bb7b031537..e521f82ba5d9 100644
> > --- a/drivers/gpu/drm/i915/intel_pipe_crc.c
> > +++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
> > @@ -34,6 +34,11 @@ static const char * const pipe_crc_sources[] = {
> > [INTEL_PIPE_CRC_SOURCE_NONE] = "none",
> > [INTEL_PIPE_CRC_SOURCE_PLANE1] = "plane1",
> > [INTEL_PIPE_CRC_SOURCE_PLANE2] = "plane2",
> > +   [INTEL_PIPE_CRC_SOURCE_PLANE3] = "plane3",
> > +   [INTEL_PIPE_CRC_SOURCE_PLANE4] = "plane4",
> > +   [INTEL_PIPE_CRC_SOURCE_PLANE5] = "plane5",
> > +   [INTEL_PIPE_CRC_SOURCE_PLANE6] = "plane6",
> > +   [INTEL_PIPE_CRC_SOURCE_PLANE7] = "plane7",
> > [INTEL_PIPE_CRC_SOURCE_PIPE] = "pipe",
> > [INTEL_PIPE_CRC_SOURCE_TV] = "TV",
> > [INTEL_PIPE_CRC_SOURCE_DP_B] = "DP-B",
> > @@ -368,6 +373,50 @@ static int ivb_pipe_crc_ctl_reg(struct 
> > drm_i915_private *dev_priv,
> > return 0;
> >  }
> >  
> > +static int skl_pipe_crc_ctl_reg(struct drm_i915_private *dev_priv,
> > +   enum pipe pipe,
> > +   enum intel_pipe_crc_source *source,
> > +   uint32_t *val,
> > +   bool set_wa)
> > +{
> > +   if (*source == INTEL_PIPE_CRC_SOURCE_AUTO)
> > +   *source = INTEL_PIPE_CRC_SOURCE_PIPE;
> > +
> > +   switch (*source) {
> > +   case INTEL_PIPE_CRC_SOURCE_PLANE1:
> > +   *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PLANE_1_SKL;
> > +   break;
> > +   case INTEL_PIPE_CRC_SOURCE_PLANE2:
> > +   *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PLANE_2_SKL;
> > +   break;
> > +   case INTEL_PIPE_CRC_SOURCE_PLANE3:
> > +   *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PLANE_3_SKL;
> > +   break;
> > +   case INTEL_PIPE_CRC_SOURCE_PLANE4:
> > +   *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PLANE_4_SKL;
> > +   break;
> > +   case INTEL_PIPE_CRC_SOURCE_PLANE5:
> > +   *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PLANE_5_SKL;
> > +   break;
> > +   case INTEL_PIPE_CRC_SOURCE_PLANE6:
> > +   *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PLANE_6_SKL;
> > +   break;
> > +   case INTEL_PIPE_CRC_SOURCE_PLANE7:
> > 

Re: [Intel-gfx] [PATCH 4/4] drm/i915: Extend skl+ crc sources with more planes

2019-02-14 Thread Rodrigo Vivi
On Thu, Feb 14, 2019 at 09:22:19PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> On skl the crc registers were extended to provide plane crcs
> for up to 7 planes. Add the new crc sources.
> 
> The current code uses the ivb+ register definitions for skl+
> which does happen to work as the plane1, plane2, and dmux/pf
> bits happen the match what ivb+ had. So no bug in the current
> code.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/i915_drv.h   |  5 ++
>  drivers/gpu/drm/i915/i915_reg.h   |  9 
>  drivers/gpu/drm/i915/intel_pipe_crc.c | 76 ++-
>  3 files changed, 88 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 4e11d970cbcf..8607c1e9ed02 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1196,6 +1196,11 @@ enum intel_pipe_crc_source {
>   INTEL_PIPE_CRC_SOURCE_NONE,
>   INTEL_PIPE_CRC_SOURCE_PLANE1,
>   INTEL_PIPE_CRC_SOURCE_PLANE2,
> + INTEL_PIPE_CRC_SOURCE_PLANE3,
> + INTEL_PIPE_CRC_SOURCE_PLANE4,
> + INTEL_PIPE_CRC_SOURCE_PLANE5,
> + INTEL_PIPE_CRC_SOURCE_PLANE6,
> + INTEL_PIPE_CRC_SOURCE_PLANE7,
>   INTEL_PIPE_CRC_SOURCE_PIPE,
>   /* TV/DP on pre-gen5/vlv can't use the pipe source. */
>   INTEL_PIPE_CRC_SOURCE_TV,
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 0df8c6e76da7..5286536e9cb8 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4017,6 +4017,15 @@ enum {
>  /* Pipe A CRC regs */
>  #define _PIPE_CRC_CTL_A  0x60050
>  #define   PIPE_CRC_ENABLE(1 << 31)
> +/* skl+ source selection */
> +#define   PIPE_CRC_SOURCE_PLANE_1_SKL(0 << 28)
> +#define   PIPE_CRC_SOURCE_PLANE_2_SKL(2 << 28)
> +#define   PIPE_CRC_SOURCE_DMUX_SKL   (4 << 28)
> +#define   PIPE_CRC_SOURCE_PLANE_3_SKL(6 << 28)
> +#define   PIPE_CRC_SOURCE_PLANE_4_SKL(7 << 28)
> +#define   PIPE_CRC_SOURCE_PLANE_5_SKL(5 << 28)
> +#define   PIPE_CRC_SOURCE_PLANE_6_SKL(3 << 28)
> +#define   PIPE_CRC_SOURCE_PLANE_7_SKL(1 << 28)

I got myself staring at spec for a while trying to
understand the logic of this sequence...


Reviewed-by: Rodrigo Vivi 



>  /* ivb+ source selection */
>  #define   PIPE_CRC_SOURCE_PRIMARY_IVB(0 << 29)
>  #define   PIPE_CRC_SOURCE_SPRITE_IVB (1 << 29)
> diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c 
> b/drivers/gpu/drm/i915/intel_pipe_crc.c
> index 66bb7b031537..e521f82ba5d9 100644
> --- a/drivers/gpu/drm/i915/intel_pipe_crc.c
> +++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
> @@ -34,6 +34,11 @@ static const char * const pipe_crc_sources[] = {
>   [INTEL_PIPE_CRC_SOURCE_NONE] = "none",
>   [INTEL_PIPE_CRC_SOURCE_PLANE1] = "plane1",
>   [INTEL_PIPE_CRC_SOURCE_PLANE2] = "plane2",
> + [INTEL_PIPE_CRC_SOURCE_PLANE3] = "plane3",
> + [INTEL_PIPE_CRC_SOURCE_PLANE4] = "plane4",
> + [INTEL_PIPE_CRC_SOURCE_PLANE5] = "plane5",
> + [INTEL_PIPE_CRC_SOURCE_PLANE6] = "plane6",
> + [INTEL_PIPE_CRC_SOURCE_PLANE7] = "plane7",
>   [INTEL_PIPE_CRC_SOURCE_PIPE] = "pipe",
>   [INTEL_PIPE_CRC_SOURCE_TV] = "TV",
>   [INTEL_PIPE_CRC_SOURCE_DP_B] = "DP-B",
> @@ -368,6 +373,50 @@ static int ivb_pipe_crc_ctl_reg(struct drm_i915_private 
> *dev_priv,
>   return 0;
>  }
>  
> +static int skl_pipe_crc_ctl_reg(struct drm_i915_private *dev_priv,
> + enum pipe pipe,
> + enum intel_pipe_crc_source *source,
> + uint32_t *val,
> + bool set_wa)
> +{
> + if (*source == INTEL_PIPE_CRC_SOURCE_AUTO)
> + *source = INTEL_PIPE_CRC_SOURCE_PIPE;
> +
> + switch (*source) {
> + case INTEL_PIPE_CRC_SOURCE_PLANE1:
> + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PLANE_1_SKL;
> + break;
> + case INTEL_PIPE_CRC_SOURCE_PLANE2:
> + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PLANE_2_SKL;
> + break;
> + case INTEL_PIPE_CRC_SOURCE_PLANE3:
> + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PLANE_3_SKL;
> + break;
> + case INTEL_PIPE_CRC_SOURCE_PLANE4:
> + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PLANE_4_SKL;
> + break;
> + case INTEL_PIPE_CRC_SOURCE_PLANE5:
> + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PLANE_5_SKL;
> + break;
> + case INTEL_PIPE_CRC_SOURCE_PLANE6:
> + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PLANE_6_SKL;
> + break;
> + case INTEL_PIPE_CRC_SOURCE_PLANE7:
> + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PLANE_7_SKL;
> + break;
> + case INTEL_PIPE_CRC_SOURCE_PIPE:
> + *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DMUX_SKL;
> + break;
> + case INTEL_PIPE_CRC_SOURCE_NONE:
> +

[Intel-gfx] [PATCH 4/4] drm/i915: Extend skl+ crc sources with more planes

2019-02-14 Thread Ville Syrjala
From: Ville Syrjälä 

On skl the crc registers were extended to provide plane crcs
for up to 7 planes. Add the new crc sources.

The current code uses the ivb+ register definitions for skl+
which does happen to work as the plane1, plane2, and dmux/pf
bits happen the match what ivb+ had. So no bug in the current
code.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/i915_drv.h   |  5 ++
 drivers/gpu/drm/i915/i915_reg.h   |  9 
 drivers/gpu/drm/i915/intel_pipe_crc.c | 76 ++-
 3 files changed, 88 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4e11d970cbcf..8607c1e9ed02 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1196,6 +1196,11 @@ enum intel_pipe_crc_source {
INTEL_PIPE_CRC_SOURCE_NONE,
INTEL_PIPE_CRC_SOURCE_PLANE1,
INTEL_PIPE_CRC_SOURCE_PLANE2,
+   INTEL_PIPE_CRC_SOURCE_PLANE3,
+   INTEL_PIPE_CRC_SOURCE_PLANE4,
+   INTEL_PIPE_CRC_SOURCE_PLANE5,
+   INTEL_PIPE_CRC_SOURCE_PLANE6,
+   INTEL_PIPE_CRC_SOURCE_PLANE7,
INTEL_PIPE_CRC_SOURCE_PIPE,
/* TV/DP on pre-gen5/vlv can't use the pipe source. */
INTEL_PIPE_CRC_SOURCE_TV,
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 0df8c6e76da7..5286536e9cb8 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4017,6 +4017,15 @@ enum {
 /* Pipe A CRC regs */
 #define _PIPE_CRC_CTL_A0x60050
 #define   PIPE_CRC_ENABLE  (1 << 31)
+/* skl+ source selection */
+#define   PIPE_CRC_SOURCE_PLANE_1_SKL  (0 << 28)
+#define   PIPE_CRC_SOURCE_PLANE_2_SKL  (2 << 28)
+#define   PIPE_CRC_SOURCE_DMUX_SKL (4 << 28)
+#define   PIPE_CRC_SOURCE_PLANE_3_SKL  (6 << 28)
+#define   PIPE_CRC_SOURCE_PLANE_4_SKL  (7 << 28)
+#define   PIPE_CRC_SOURCE_PLANE_5_SKL  (5 << 28)
+#define   PIPE_CRC_SOURCE_PLANE_6_SKL  (3 << 28)
+#define   PIPE_CRC_SOURCE_PLANE_7_SKL  (1 << 28)
 /* ivb+ source selection */
 #define   PIPE_CRC_SOURCE_PRIMARY_IVB  (0 << 29)
 #define   PIPE_CRC_SOURCE_SPRITE_IVB   (1 << 29)
diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c 
b/drivers/gpu/drm/i915/intel_pipe_crc.c
index 66bb7b031537..e521f82ba5d9 100644
--- a/drivers/gpu/drm/i915/intel_pipe_crc.c
+++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
@@ -34,6 +34,11 @@ static const char * const pipe_crc_sources[] = {
[INTEL_PIPE_CRC_SOURCE_NONE] = "none",
[INTEL_PIPE_CRC_SOURCE_PLANE1] = "plane1",
[INTEL_PIPE_CRC_SOURCE_PLANE2] = "plane2",
+   [INTEL_PIPE_CRC_SOURCE_PLANE3] = "plane3",
+   [INTEL_PIPE_CRC_SOURCE_PLANE4] = "plane4",
+   [INTEL_PIPE_CRC_SOURCE_PLANE5] = "plane5",
+   [INTEL_PIPE_CRC_SOURCE_PLANE6] = "plane6",
+   [INTEL_PIPE_CRC_SOURCE_PLANE7] = "plane7",
[INTEL_PIPE_CRC_SOURCE_PIPE] = "pipe",
[INTEL_PIPE_CRC_SOURCE_TV] = "TV",
[INTEL_PIPE_CRC_SOURCE_DP_B] = "DP-B",
@@ -368,6 +373,50 @@ static int ivb_pipe_crc_ctl_reg(struct drm_i915_private 
*dev_priv,
return 0;
 }
 
+static int skl_pipe_crc_ctl_reg(struct drm_i915_private *dev_priv,
+   enum pipe pipe,
+   enum intel_pipe_crc_source *source,
+   uint32_t *val,
+   bool set_wa)
+{
+   if (*source == INTEL_PIPE_CRC_SOURCE_AUTO)
+   *source = INTEL_PIPE_CRC_SOURCE_PIPE;
+
+   switch (*source) {
+   case INTEL_PIPE_CRC_SOURCE_PLANE1:
+   *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PLANE_1_SKL;
+   break;
+   case INTEL_PIPE_CRC_SOURCE_PLANE2:
+   *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PLANE_2_SKL;
+   break;
+   case INTEL_PIPE_CRC_SOURCE_PLANE3:
+   *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PLANE_3_SKL;
+   break;
+   case INTEL_PIPE_CRC_SOURCE_PLANE4:
+   *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PLANE_4_SKL;
+   break;
+   case INTEL_PIPE_CRC_SOURCE_PLANE5:
+   *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PLANE_5_SKL;
+   break;
+   case INTEL_PIPE_CRC_SOURCE_PLANE6:
+   *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PLANE_6_SKL;
+   break;
+   case INTEL_PIPE_CRC_SOURCE_PLANE7:
+   *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_PLANE_7_SKL;
+   break;
+   case INTEL_PIPE_CRC_SOURCE_PIPE:
+   *val = PIPE_CRC_ENABLE | PIPE_CRC_SOURCE_DMUX_SKL;
+   break;
+   case INTEL_PIPE_CRC_SOURCE_NONE:
+   *val = 0;
+   break;
+   default:
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
 static int get_new_crc_ctl_reg(struct drm_i915_private *dev_priv,
   enum pipe pipe,
   enum intel_pipe_crc_source *source, u32 *val,
@@ -381,8 +430,10 @@ static i