[PATCH 0/4] drm/i915: Fix a few code checker errors

[PATCH 0/4] drm/i915: Fix a few code checker errors

[Imre Deak]

Fix a few minor issues pointed at by Coverity.

Imre Deak (4):
  drm/i915/gen9: Avoid using negative array index in skl_update_plane()
  drm/i915: Add comments about fixed pipe->transcoder/PLL mapping
  drm/i915: Handle error return from dma_set_coherent_mask()
  drm/i915: Remove redundant const from function return type

 drivers/gpu/drm/i915/i915_debugfs.c  | 10 +++++-----
 drivers/gpu/drm/i915/i915_dma.c      | 22 ++++++++++++++++++----
 drivers/gpu/drm/i915/intel_display.c |  5 +++++
 drivers/gpu/drm/i915/intel_sprite.c  |  6 ++++--
 4 files changed, 32 insertions(+), 11 deletions(-)

-- 
2.5.0

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

[PATCH 1/4] drm/i915/gen9: Avoid using negative array index in skl_update_plane()

[Imre Deak]

scaler_id may be negative as shown by conditions later in the function,
so don't use it as an array index in that case.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_sprite.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 0f3e230..286bcc6 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -203,8 +203,6 @@ skl_update_plane(struct drm_plane *drm_plane,
 	uint32_t y = plane_state->src.y1 >> 16;
 	uint32_t src_w = drm_rect_width(&plane_state->src) >> 16;
 	uint32_t src_h = drm_rect_height(&plane_state->src) >> 16;
-	const struct intel_scaler *scaler =
-		&crtc_state->scaler_state.scalers[plane_state->scaler_id];
 
 	plane_ctl = PLANE_CTL_ENABLE |
 		PLANE_CTL_PIPE_GAMMA_ENABLE |
@@ -262,10 +260,14 @@ skl_update_plane(struct drm_plane *drm_plane,
 	if (plane_state->scaler_id >= 0) {
 		uint32_t ps_ctrl = 0;
 		int scaler_id = plane_state->scaler_id;
+		const struct intel_scaler *scaler;
 
 		DRM_DEBUG_KMS("plane = %d PS_PLANE_SEL(plane) = 0x%x\n", plane,
 			PS_PLANE_SEL(plane));
+
+		scaler = &crtc_state->scaler_state.scalers[scaler_id];
 		ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(plane) | scaler->mode;
+
 		I915_WRITE(SKL_PS_CTRL(pipe, scaler_id), ps_ctrl);
 		I915_WRITE(SKL_PS_PWR_GATE(pipe, scaler_id), 0);
 		I915_WRITE(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x << 16) | crtc_y);
-- 
2.5.0

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

[PATCH 2/4] drm/i915: Add comments about fixed pipe->transcoder/PLL mapping

[Imre Deak]

Code checkers may complain about the explicit casts between different
enum types, so add comments for known-valid cases to help future
triaging of such complaints.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b60d9b6..763726d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9294,6 +9294,7 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
 		ironlake_get_fdi_m_n_config(crtc, pipe_config);
 
 		if (HAS_PCH_IBX(dev_priv)) {
+			/* the pipe->pll mapping is fixed */
 			pll_id = (enum intel_dpll_id) crtc->pipe;
 		} else {
 			tmp = I915_READ(PCH_DPLL_SEL);
@@ -9844,6 +9845,10 @@ static bool hsw_get_transcoder_state(struct intel_crtc *crtc,
 	enum intel_display_power_domain power_domain;
 	u32 tmp;
 
+	/*
+	 * The transcoder->pipe mapping is fixed with the exception of the eDP
+	 * transcoder handled below.
+	 */
 	pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe;
 
 	/*
-- 
2.5.0

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

[PATCH 3/4] drm/i915: Handle error return from dma_set_coherent_mask()

[Imre Deak]

A failure from these functions can lead to obscure bugs later, so it's
better not to suppress them and just fail module loading.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 0eadeb6..a7fa82e 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1262,8 +1262,15 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
 	pci_set_master(dev->pdev);
 
 	/* overlay on gen2 is broken and can't address above 1G */
-	if (IS_GEN2(dev))
-		dma_set_coherent_mask(&dev->pdev->dev, DMA_BIT_MASK(30));
+	if (IS_GEN2(dev)) {
+		ret = dma_set_coherent_mask(&dev->pdev->dev, DMA_BIT_MASK(30));
+		if (ret) {
+			DRM_ERROR("failed to set DMA mask\n");
+
+			goto out_ggtt;
+		}
+	}
+
 
 	/* 965GM sometimes incorrectly writes to hardware status page (HWS)
 	 * using 32bit addressing, overwriting memory if HWS is located
@@ -1273,8 +1280,15 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
 	 * behaviour if any general state is accessed within a page above 4GB,
 	 * which also needs to be handled carefully.
 	 */
-	if (IS_BROADWATER(dev) || IS_CRESTLINE(dev))
-		dma_set_coherent_mask(&dev->pdev->dev, DMA_BIT_MASK(32));
+	if (IS_BROADWATER(dev) || IS_CRESTLINE(dev)) {
+		ret = dma_set_coherent_mask(&dev->pdev->dev, DMA_BIT_MASK(32));
+
+		if (ret) {
+			DRM_ERROR("failed to set DMA mask\n");
+
+			goto out_ggtt;
+		}
+	}
 
 	aperture_size = ggtt->mappable_end;
 
-- 
2.5.0

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

[PATCH 4/4] drm/i915: Remove redundant const from function return type

[Imre Deak]

Marking function return types as const is redundant, as these are
rvalues and as such constant by definition. Code checkers and GCC will
warn about this so remove the modifier.

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 5e5b5cc..24f4105 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -89,17 +89,17 @@ static int i915_capabilities(struct seq_file *m, void *data)
 	return 0;
 }
 
-static const char get_active_flag(struct drm_i915_gem_object *obj)
+static char get_active_flag(struct drm_i915_gem_object *obj)
 {
 	return obj->active ? '*' : ' ';
 }
 
-static const char get_pin_flag(struct drm_i915_gem_object *obj)
+static char get_pin_flag(struct drm_i915_gem_object *obj)
 {
 	return obj->pin_display ? 'p' : ' ';
 }
 
-static const char get_tiling_flag(struct drm_i915_gem_object *obj)
+static char get_tiling_flag(struct drm_i915_gem_object *obj)
 {
 	switch (obj->tiling_mode) {
 	default:
@@ -109,12 +109,12 @@ static const char get_tiling_flag(struct drm_i915_gem_object *obj)
 	}
 }
 
-static inline const char get_global_flag(struct drm_i915_gem_object *obj)
+static char get_global_flag(struct drm_i915_gem_object *obj)
 {
 	return i915_gem_obj_to_ggtt(obj) ? 'g' : ' ';
 }
 
-static inline const char get_pin_mapped_flag(struct drm_i915_gem_object *obj)
+static char get_pin_mapped_flag(struct drm_i915_gem_object *obj)
 {
 	return obj->mapping ? 'M' : ' ';
 }
-- 
2.5.0

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

Re: [PATCH 1/4] drm/i915/gen9: Avoid using negative array index in skl_update_plane()

[Ville Syrjälä]

On Thu, May 12, 2016 at 04:18:49PM +0300, Imre Deak wrote:
> scaler_id may be negative as shown by conditions later in the function,
> so don't use it as an array index in that case.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_sprite.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 0f3e230..286bcc6 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -203,8 +203,6 @@ skl_update_plane(struct drm_plane *drm_plane,
>  	uint32_t y = plane_state->src.y1 >> 16;
>  	uint32_t src_w = drm_rect_width(&plane_state->src) >> 16;
>  	uint32_t src_h = drm_rect_height(&plane_state->src) >> 16;
> -	const struct intel_scaler *scaler =
> -		&crtc_state->scaler_state.scalers[plane_state->scaler_id];
>  
>  	plane_ctl = PLANE_CTL_ENABLE |
>  		PLANE_CTL_PIPE_GAMMA_ENABLE |
> @@ -262,10 +260,14 @@ skl_update_plane(struct drm_plane *drm_plane,
>  	if (plane_state->scaler_id >= 0) {
>  		uint32_t ps_ctrl = 0;
>  		int scaler_id = plane_state->scaler_id;
> +		const struct intel_scaler *scaler;
>  
>  		DRM_DEBUG_KMS("plane = %d PS_PLANE_SEL(plane) = 0x%x\n", plane,
>  			PS_PLANE_SEL(plane));
> +
> +		scaler = &crtc_state->scaler_state.scalers[scaler_id];
>  		ps_ctrl = PS_SCALER_EN | PS_PLANE_SEL(plane) | scaler->mode;

Could nuke ps_ctrl while at it perhaps.

But in any case 
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +
>  		I915_WRITE(SKL_PS_CTRL(pipe, scaler_id), ps_ctrl);
>  		I915_WRITE(SKL_PS_PWR_GATE(pipe, scaler_id), 0);
>  		I915_WRITE(SKL_PS_WIN_POS(pipe, scaler_id), (crtc_x << 16) | crtc_y);
> -- 
> 2.5.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 2/4] drm/i915: Add comments about fixed pipe->transcoder/PLL mapping

[Ville Syrjälä]

On Thu, May 12, 2016 at 04:18:50PM +0300, Imre Deak wrote:
> Code checkers may complain about the explicit casts between different
> enum types, so add comments for known-valid cases to help future
> triaging of such complaints.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b60d9b6..763726d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9294,6 +9294,7 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
>  		ironlake_get_fdi_m_n_config(crtc, pipe_config);
>  
>  		if (HAS_PCH_IBX(dev_priv)) {
> +			/* the pipe->pll mapping is fixed */

Should perhaps say 'pch transocder->pll mapping' here.

>  			pll_id = (enum intel_dpll_id) crtc->pipe;
>  		} else {
>  			tmp = I915_READ(PCH_DPLL_SEL);
> @@ -9844,6 +9845,10 @@ static bool hsw_get_transcoder_state(struct intel_crtc *crtc,
>  	enum intel_display_power_domain power_domain;
>  	u32 tmp;
>  
> +	/*
> +	 * The transcoder->pipe mapping is fixed with the exception of the eDP

Saying 'pipe->transcoder mapping' would feel more natural to me
since that's the way the data flows. Also that's the way assignment in
the code happens.

But I can live with the current thing too, so
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +	 * transcoder handled below.
> +	 */
>  	pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe;
>  
>  	/*
> -- 
> 2.5.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 4/4] drm/i915: Remove redundant const from function return type

[Ville Syrjälä]

On Thu, May 12, 2016 at 04:18:52PM +0300, Imre Deak wrote:
> Marking function return types as const is redundant, as these are
> rvalues and as such constant by definition. Code checkers and GCC will
> warn about this so remove the modifier.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 5e5b5cc..24f4105 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -89,17 +89,17 @@ static int i915_capabilities(struct seq_file *m, void *data)
>  	return 0;
>  }
>  
> -static const char get_active_flag(struct drm_i915_gem_object *obj)
> +static char get_active_flag(struct drm_i915_gem_object *obj)
>  {
>  	return obj->active ? '*' : ' ';
>  }
>  
> -static const char get_pin_flag(struct drm_i915_gem_object *obj)
> +static char get_pin_flag(struct drm_i915_gem_object *obj)
>  {
>  	return obj->pin_display ? 'p' : ' ';
>  }
>  
> -static const char get_tiling_flag(struct drm_i915_gem_object *obj)
> +static char get_tiling_flag(struct drm_i915_gem_object *obj)
>  {
>  	switch (obj->tiling_mode) {
>  	default:
> @@ -109,12 +109,12 @@ static const char get_tiling_flag(struct drm_i915_gem_object *obj)
>  	}
>  }
>  
> -static inline const char get_global_flag(struct drm_i915_gem_object *obj)
> +static char get_global_flag(struct drm_i915_gem_object *obj)
>  {
>  	return i915_gem_obj_to_ggtt(obj) ? 'g' : ' ';
>  }
>  
> -static inline const char get_pin_mapped_flag(struct drm_i915_gem_object *obj)
> +static char get_pin_mapped_flag(struct drm_i915_gem_object *obj)
>  {
>  	return obj->mapping ? 'M' : ' ';
>  }
> -- 
> 2.5.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 3/4] drm/i915: Handle error return from dma_set_coherent_mask()

[Ville Syrjälä]

On Thu, May 12, 2016 at 04:18:51PM +0300, Imre Deak wrote:
> A failure from these functions can lead to obscure bugs later, so it's
> better not to suppress them and just fail module loading.

Based on a cursory examination looks like this should never fail, but I
suppose having the error handling won't hurt, so

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 0eadeb6..a7fa82e 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1262,8 +1262,15 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
>  	pci_set_master(dev->pdev);
>  
>  	/* overlay on gen2 is broken and can't address above 1G */
> -	if (IS_GEN2(dev))
> -		dma_set_coherent_mask(&dev->pdev->dev, DMA_BIT_MASK(30));
> +	if (IS_GEN2(dev)) {
> +		ret = dma_set_coherent_mask(&dev->pdev->dev, DMA_BIT_MASK(30));
> +		if (ret) {
> +			DRM_ERROR("failed to set DMA mask\n");
> +
> +			goto out_ggtt;
> +		}
> +	}
> +
>  
>  	/* 965GM sometimes incorrectly writes to hardware status page (HWS)
>  	 * using 32bit addressing, overwriting memory if HWS is located
> @@ -1273,8 +1280,15 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
>  	 * behaviour if any general state is accessed within a page above 4GB,
>  	 * which also needs to be handled carefully.
>  	 */
> -	if (IS_BROADWATER(dev) || IS_CRESTLINE(dev))
> -		dma_set_coherent_mask(&dev->pdev->dev, DMA_BIT_MASK(32));
> +	if (IS_BROADWATER(dev) || IS_CRESTLINE(dev)) {
> +		ret = dma_set_coherent_mask(&dev->pdev->dev, DMA_BIT_MASK(32));
> +
> +		if (ret) {
> +			DRM_ERROR("failed to set DMA mask\n");
> +
> +			goto out_ggtt;
> +		}
> +	}
>  
>  	aperture_size = ggtt->mappable_end;
>  
> -- 
> 2.5.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH 2/4] drm/i915: Add comments about fixed pipe->transcoder/PLL mapping

[Imre Deak]

On Thu, 2016-05-12 at 16:30 +0300, Ville Syrjälä wrote:
> On Thu, May 12, 2016 at 04:18:50PM +0300, Imre Deak wrote:
> > Code checkers may complain about the explicit casts between different
> > enum types, so add comments for known-valid cases to help future
> > triaging of such complaints.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index b60d9b6..763726d 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -9294,6 +9294,7 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
> >  		ironlake_get_fdi_m_n_config(crtc, pipe_config);
> >  
> >  		if (HAS_PCH_IBX(dev_priv)) {
> > +			/* the pipe->pll mapping is fixed */
> 
> Should perhaps say 'pch transocder->pll mapping' here.

Ok, but then 'pipe->pch transcoder and pch transcoder->pll mapping is
fixed'?

> >  			pll_id = (enum intel_dpll_id) crtc->pipe;
> >  		} else {
> >  			tmp = I915_READ(PCH_DPLL_SEL);
> > @@ -9844,6 +9845,10 @@ static bool hsw_get_transcoder_state(struct intel_crtc *crtc,
> >  	enum intel_display_power_domain power_domain;
> >  	u32 tmp;
> >  
> > +	/*
> > +	 * The transcoder->pipe mapping is fixed with the exception of the eDP
> 
> Saying 'pipe->transcoder mapping' would feel more natural to me
> since that's the way the data flows. Also that's the way assignment in
> the code happens.

Ok, will change that.

> But I can live with the current thing too, so
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> > +	 * transcoder handled below.
> > +	 */
> >  	pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe;
> >  
> >  	/*
> > -- 
> > 2.5.0
> > 
> > _______________________________________________
> > 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

Re: [PATCH 2/4] drm/i915: Add comments about fixed pipe->transcoder/PLL mapping

[Ville Syrjälä]

On Thu, May 12, 2016 at 05:00:56PM +0300, Imre Deak wrote:
> On Thu, 2016-05-12 at 16:30 +0300, Ville Syrjälä wrote:
> > On Thu, May 12, 2016 at 04:18:50PM +0300, Imre Deak wrote:
> > > Code checkers may complain about the explicit casts between different
> > > enum types, so add comments for known-valid cases to help future
> > > triaging of such complaints.
> > > 
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > > index b60d9b6..763726d 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -9294,6 +9294,7 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
> > >  		ironlake_get_fdi_m_n_config(crtc, pipe_config);
> > >  
> > >  		if (HAS_PCH_IBX(dev_priv)) {
> > > +			/* the pipe->pll mapping is fixed */
> > 
> > Should perhaps say 'pch transocder->pll mapping' here.
> 
> Ok, but then 'pipe->pch transcoder and pch transcoder->pll mapping is
> fixed'?

I guess you could say that. We don't have a separate enum for pch
transcoder since it's the same as the pipe always, and thus we usually
use 'pipe' to denote the pch transcoder as well. Well, except on
HSW/BDW which only have pch transcoder A, and sometimes on those
platforms we use enum transcoder also for the pch transcoder.
So it's a huge mess really, and I was considering adding a pch
transcoder enum at some point but so far I didn't get bored enough
to actually do it.

> 
> > >  			pll_id = (enum intel_dpll_id) crtc->pipe;
> > >  		} else {
> > >  			tmp = I915_READ(PCH_DPLL_SEL);
> > > @@ -9844,6 +9845,10 @@ static bool hsw_get_transcoder_state(struct intel_crtc *crtc,
> > >  	enum intel_display_power_domain power_domain;
> > >  	u32 tmp;
> > >  
> > > +	/*
> > > +	 * The transcoder->pipe mapping is fixed with the exception of the eDP
> > 
> > Saying 'pipe->transcoder mapping' would feel more natural to me
> > since that's the way the data flows. Also that's the way assignment in
> > the code happens.
> 
> Ok, will change that.
> 
> > But I can live with the current thing too, so
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > > +	 * transcoder handled below.
> > > +	 */
> > >  	pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe;
> > >  
> > >  	/*
> > > -- 
> > > 2.5.0
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 

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

✗ Ro.CI.BAT: failure for drm/i915: Fix a few code checker errors

[Patchwork]

== Series Details ==

Series: drm/i915: Fix a few code checker errors
URL   : https://patchwork.freedesktop.org/series/7071/
State : failure

== Summary ==

Series 7071v1 drm/i915: Fix a few code checker errors
http://patchwork.freedesktop.org/api/1.0/series/7071/revisions/1/mbox

Test drv_hangman:
        Subgroup error-state-basic:
                fail       -> PASS       (ro-ilk1-i5-650)
Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-cmd:
                pass       -> FAIL       (fi-byt-n2820)
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                fail       -> PASS       (ro-byt-n2820)
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-c:
                pass       -> SKIP       (fi-skl-i5-6260u)
        Subgroup read-crc-pipe-b-frame-sequence:
                skip       -> PASS       (fi-skl-i5-6260u)
        Subgroup read-crc-pipe-c:
                skip       -> PASS       (fi-skl-i5-6260u)

fi-byt-n2820     total:218  pass:174  dwarn:0   dfail:0   fail:3   skip:41 
fi-hsw-i7-4770k  total:219  pass:197  dwarn:0   dfail:0   fail:1   skip:21 
fi-hsw-i7-4770r  total:219  pass:193  dwarn:0   dfail:0   fail:0   skip:26 
fi-kbl-y         total:219  pass:189  dwarn:3   dfail:0   fail:2   skip:25 
fi-skl-i5-6260u  total:219  pass:206  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-i7-6700k  total:219  pass:191  dwarn:0   dfail:0   fail:0   skip:28 
ro-bdw-i5-5250u  total:219  pass:181  dwarn:0   dfail:0   fail:0   skip:38 
ro-bdw-i7-5557U  total:219  pass:206  dwarn:0   dfail:0   fail:0   skip:13 
ro-bdw-i7-5600u  total:219  pass:187  dwarn:0   dfail:0   fail:0   skip:32 
ro-bsw-n3050     total:219  pass:175  dwarn:0   dfail:0   fail:2   skip:42 
ro-byt-n2820     total:218  pass:174  dwarn:0   dfail:0   fail:3   skip:41 
ro-hsw-i3-4010u  total:218  pass:193  dwarn:0   dfail:0   fail:0   skip:25 
ro-hsw-i7-4770r  total:219  pass:194  dwarn:0   dfail:0   fail:0   skip:25 
ro-ilk-i7-620lm  total:219  pass:151  dwarn:0   dfail:0   fail:1   skip:67 
ro-ilk1-i5-650   total:214  pass:152  dwarn:0   dfail:0   fail:1   skip:61 
ro-ivb-i7-3770   total:219  pass:183  dwarn:0   dfail:0   fail:0   skip:36 
ro-ivb2-i7-3770  total:219  pass:186  dwarn:0   dfail:0   fail:1   skip:32 
ro-skl-i7-6700hq total:214  pass:189  dwarn:0   dfail:0   fail:0   skip:25 
ro-snb-i7-2620M  total:219  pass:177  dwarn:0   dfail:0   fail:1   skip:41 
fi-bdw-i7-5557u failed to connect after reboot
fi-bsw-n3050 failed to connect after reboot
fi-snb-i7-2600 failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_864/

86821b8 drm-intel-nightly: 2016y-05m-12d-14h-21m-45s UTC integration manifest
3d5079e drm/i915: Remove redundant const from function return type
fbd4b03 drm/i915: Handle error return from dma_set_coherent_mask()
25f4fc4 drm/i915: Add comments about fixed pipe->transcoder/PLL mapping
040f9ba drm/i915/gen9: Avoid using negative array index in skl_update_plane()

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

Re: ✗ Ro.CI.BAT: failure for drm/i915: Fix a few code checker errors

[Imre Deak]

On Thu, 2016-05-12 at 15:31 +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Fix a few code checker errors
> URL   : https://patchwork.freedesktop.org/series/7071/
> State : failure
> 
> == Summary ==
> 
> Series 7071v1 drm/i915: Fix a few code checker errors
> http://patchwork.freedesktop.org/api/1.0/series/7071/revisions/1/mbox
> 
> Test drv_hangman:
>         Subgroup error-state-basic:
>                 fail       -> PASS       (ro-ilk1-i5-650)
> Test gem_exec_flush:
>         Subgroup basic-batch-kernel-default-cmd:
>                 pass       -> FAIL       (fi-byt-n2820)

Unrelated, pre-existing issue:
https://bugs.freedesktop.org/show_bug.cgi?id=95372

(gem_exec_flush:8135) ioctl-wrappers-CRITICAL: Test assertion failure function gem_execbuf, file ioctl_wrappers.c:589:
(gem_exec_flush:8135) ioctl-wrappers-CRITICAL: Failed assertion: __gem_execbuf(fd, execbuf) == 0
(gem_exec_flush:8135) ioctl-wrappers-CRITICAL: error: -22 != 0

Thanks for the review, I pushed the patches with your comments
addressed to dinq.

--Imre

> Test kms_flip:
>         Subgroup basic-flip-vs-wf_vblank:
>                 fail       -> PASS       (ro-byt-n2820)
> Test kms_pipe_crc_basic:
>         Subgroup hang-read-crc-pipe-c:
>                 pass       -> SKIP       (fi-skl-i5-6260u)
>         Subgroup read-crc-pipe-b-frame-sequence:
>                 skip       -> PASS       (fi-skl-i5-6260u)
>         Subgroup read-crc-pipe-c:
>                 skip       -> PASS       (fi-skl-i5-6260u)
> 
> fi-byt-
> n2820     total:218  pass:174  dwarn:0   dfail:0   fail:3   skip:41 
> fi-hsw-i7-
> 4770k  total:219  pass:197  dwarn:0   dfail:0   fail:1   skip:21 
> fi-hsw-i7-
> 4770r  total:219  pass:193  dwarn:0   dfail:0   fail:0   skip:26 
> fi-kbl-
> y         total:219  pass:189  dwarn:3   dfail:0   fail:2   skip:25 
> fi-skl-i5-
> 6260u  total:219  pass:206  dwarn:0   dfail:0   fail:0   skip:13 
> fi-skl-i7-
> 6700k  total:219  pass:191  dwarn:0   dfail:0   fail:0   skip:28 
> ro-bdw-i5-
> 5250u  total:219  pass:181  dwarn:0   dfail:0   fail:0   skip:38 
> ro-bdw-i7-
> 5557U  total:219  pass:206  dwarn:0   dfail:0   fail:0   skip:13 
> ro-bdw-i7-
> 5600u  total:219  pass:187  dwarn:0   dfail:0   fail:0   skip:32 
> ro-bsw-
> n3050     total:219  pass:175  dwarn:0   dfail:0   fail:2   skip:42 
> ro-byt-
> n2820     total:218  pass:174  dwarn:0   dfail:0   fail:3   skip:41 
> ro-hsw-i3-
> 4010u  total:218  pass:193  dwarn:0   dfail:0   fail:0   skip:25 
> ro-hsw-i7-
> 4770r  total:219  pass:194  dwarn:0   dfail:0   fail:0   skip:25 
> ro-ilk-i7-
> 620lm  total:219  pass:151  dwarn:0   dfail:0   fail:1   skip:67 
> ro-ilk1-i5-
> 650   total:214  pass:152  dwarn:0   dfail:0   fail:1   skip:61 
> ro-ivb-i7-
> 3770   total:219  pass:183  dwarn:0   dfail:0   fail:0   skip:36 
> ro-ivb2-i7-
> 3770  total:219  pass:186  dwarn:0   dfail:0   fail:1   skip:32 
> ro-skl-i7-6700hq
> total:214  pass:189  dwarn:0   dfail:0   fail:0   skip:25 
> ro-snb-i7-
> 2620M  total:219  pass:177  dwarn:0   dfail:0   fail:1   skip:41 
> fi-bdw-i7-5557u failed to connect after reboot
> fi-bsw-n3050 failed to connect after reboot
> fi-snb-i7-2600 failed to connect after reboot
> 
> Results at /archive/results/CI_IGT_test/RO_Patchwork_864/
> 
> 86821b8 drm-intel-nightly: 2016y-05m-12d-14h-21m-45s UTC integration
> manifest
> 3d5079e drm/i915: Remove redundant const from function return type
> fbd4b03 drm/i915: Handle error return from dma_set_coherent_mask()
> 25f4fc4 drm/i915: Add comments about fixed pipe->transcoder/PLL
> mapping
> 040f9ba drm/i915/gen9: Avoid using negative array index in
> skl_update_plane()
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Ro.CI.BAT: failure for drm/i915: Fix a few code checker errors

[Chris Wilson]

On Fri, May 13, 2016 at 03:24:32PM +0300, Imre Deak wrote:
> On Thu, 2016-05-12 at 15:31 +0000, Patchwork wrote:
> > == Series Details ==
> > 
> > Series: drm/i915: Fix a few code checker errors
> > URL   : https://patchwork.freedesktop.org/series/7071/
> > State : failure
> > 
> > == Summary ==
> > 
> > Series 7071v1 drm/i915: Fix a few code checker errors
> > http://patchwork.freedesktop.org/api/1.0/series/7071/revisions/1/mbox
> > 
> > Test drv_hangman:
> >         Subgroup error-state-basic:
> >                 fail       -> PASS       (ro-ilk1-i5-650)
> > Test gem_exec_flush:
> >         Subgroup basic-batch-kernel-default-cmd:
> >                 pass       -> FAIL       (fi-byt-n2820)
> 
> Unrelated, pre-existing issue:
> https://bugs.freedesktop.org/show_bug.cgi?id=95372

Fwiw, there are at least a coupleo of issues in gem_exec_flush for byt
that require different fixes (the cmdparser is a different
synchronisation issue). And has been accumulating the occasional
intel_do_flush_locked EINVAL bug report.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx