Re: [Intel-gfx] [PATCH 03/19] drm/i915: Make HAS_GMCH_DISPLAY only take dev_priv

[David Weinehall]

On Wed, Oct 12, 2016 at 09:43:02AM +0100, Tvrtko Ursulin wrote:
> 
> On 12/10/2016 09:17, David Weinehall wrote:
> > On Tue, Oct 11, 2016 at 02:21:36PM +0100, Tvrtko Ursulin wrote:
> > > From: Tvrtko Ursulin
> > > 
> > > More .rodata string saving by avoid __I915__ magic inside WARNs.
> > > 
> > > v2: Add parantheses around dev_priv. (Ville Syrjala)
> > > 
> > > Signed-off-by: Tvrtko Ursulin
> > Reviewed-by: David Weinehall
> > 
> > Note that once this patch series goes in (or before),
> > we should have a patch that turns intel_hdmi_to_dev() into
> > intel_hdmi_to_dev_priv().  If you look at the code in
> > intel_hdmi.c, almost every (after the dev -> dev_priv transition
> > I think it's every) instance where it's used converts
> > dev immediately further to dev_priv.
> 
> Agreed, but best left for later I think. And there is more of those
> opportunities throughout the code which I spotted while doing this.
> 
> Regards,
> 
> Tvrtko
> 
> P.S. For some reason reply to all from thunderbird keeps dropping you from
> the recipients. I might forget to manually add you.

That's because my e-mail client sets the Mail-Followup-To header;
I'm subscribed to the list -- I don't need duplicate copies addressed
directly to me.


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

Re: [Intel-gfx] [PATCH 03/19] drm/i915: Make HAS_GMCH_DISPLAY only take dev_priv

[Tvrtko Ursulin]

On 12/10/2016 09:17, David Weinehall wrote:

On Tue, Oct 11, 2016 at 02:21:36PM +0100, Tvrtko Ursulin wrote:

From: Tvrtko Ursulin

More .rodata string saving by avoid __I915__ magic inside WARNs.

v2: Add parantheses around dev_priv. (Ville Syrjala)

Signed-off-by: Tvrtko Ursulin

Reviewed-by: David Weinehall

Note that once this patch series goes in (or before),
we should have a patch that turns intel_hdmi_to_dev() into
intel_hdmi_to_dev_priv().  If you look at the code in
intel_hdmi.c, almost every (after the dev -> dev_priv transition
I think it's every) instance where it's used converts
dev immediately further to dev_priv.


Agreed, but best left for later I think. And there is more of those 
opportunities throughout the code which I spotted while doing this.


Regards,

Tvrtko

P.S. For some reason reply to all from thunderbird keeps dropping you 
from the recipients. I might forget to manually add you.



---
  drivers/gpu/drm/i915/i915_drv.h| 2 +-
  drivers/gpu/drm/i915/intel_color.c | 6 +++---
  drivers/gpu/drm/i915/intel_display.c   | 8 
  drivers/gpu/drm/i915/intel_dp.c| 2 +-
  drivers/gpu/drm/i915/intel_dsi.c   | 2 +-
  drivers/gpu/drm/i915/intel_fifo_underrun.c | 2 +-
  drivers/gpu/drm/i915/intel_hdmi.c  | 5 +++--
  7 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3caa1c767512..1a4698e665be 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2863,7 +2863,7 @@ struct drm_i915_cmd_table {
  #define HAS_PCH_NOP(dev_priv) (INTEL_PCH_TYPE(dev_priv) == PCH_NOP)
  #define HAS_PCH_SPLIT(dev_priv) (INTEL_PCH_TYPE(dev_priv) != PCH_NONE)
  
-#define HAS_GMCH_DISPLAY(dev) (INTEL_INFO(dev)->has_gmch_display)

+#define HAS_GMCH_DISPLAY(dev_priv) ((dev_priv)->info.has_gmch_display)
  
  /* DPF == dynamic parity feature */

  #define HAS_L3_DPF(dev) (INTEL_INFO(dev)->has_l3_dpf)
diff --git a/drivers/gpu/drm/i915/intel_color.c 
b/drivers/gpu/drm/i915/intel_color.c
index 95a72771eea6..5362c07932d3 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -273,7 +273,7 @@ static void i9xx_load_luts_internal(struct drm_crtc *crtc,
enum pipe pipe = intel_crtc->pipe;
int i;
  
-	if (HAS_GMCH_DISPLAY(dev)) {

+   if (HAS_GMCH_DISPLAY(dev_priv)) {
if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI))
assert_dsi_pll_enabled(dev_priv);
else
@@ -288,7 +288,7 @@ static void i9xx_load_luts_internal(struct drm_crtc *crtc,
(drm_color_lut_extract(lut[i].green, 8) << 8) |
drm_color_lut_extract(lut[i].blue, 8);
  
-			if (HAS_GMCH_DISPLAY(dev))

+   if (HAS_GMCH_DISPLAY(dev_priv))
I915_WRITE(PALETTE(pipe, i), word);
else
I915_WRITE(LGC_PALETTE(pipe, i), word);
@@ -297,7 +297,7 @@ static void i9xx_load_luts_internal(struct drm_crtc *crtc,
for (i = 0; i < 256; i++) {
uint32_t word = (i << 16) | (i << 8) | i;
  
-			if (HAS_GMCH_DISPLAY(dev))

+   if (HAS_GMCH_DISPLAY(dev_priv))
I915_WRITE(PALETTE(pipe, i), word);
else
I915_WRITE(LGC_PALETTE(pipe, i), word);
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 0a69e80821ee..b7685936d324 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5036,7 +5036,7 @@ intel_pre_disable_primary_noatomic(struct drm_crtc *crtc)
 * event which is after the vblank start event, so we need to have a
 * wait-for-vblank between disabling the plane and the pipe.
 */
-   if (HAS_GMCH_DISPLAY(dev)) {
+   if (HAS_GMCH_DISPLAY(dev_priv)) {
intel_set_memory_cxsr(dev_priv, false);
dev_priv->wm.vlv.cxsr = false;
intel_wait_for_vblank(dev, pipe);
@@ -5101,7 +5101,7 @@ static void intel_pre_plane_update(struct 
intel_crtc_state *old_crtc_state)
intel_pre_disable_primary(>base);
}
  
-	if (pipe_config->disable_cxsr && HAS_GMCH_DISPLAY(dev)) {

+   if (pipe_config->disable_cxsr && HAS_GMCH_DISPLAY(dev_priv)) {
crtc->wm.cxsr_allowed = false;
  
  		/*

@@ -10895,7 +10895,7 @@ static void intel_crtc_update_cursor(struct drm_crtc 
*crtc,
pos |= y << CURSOR_Y_SHIFT;
  
  		/* ILK+ do this automagically */

-   if (HAS_GMCH_DISPLAY(dev) &&
+   if (HAS_GMCH_DISPLAY(dev_priv) &&
plane_state->base.rotation == DRM_ROTATE_180) {
base += 

Re: [Intel-gfx] [PATCH 03/19] drm/i915: Make HAS_GMCH_DISPLAY only take dev_priv

[David Weinehall]

On Tue, Oct 11, 2016 at 02:21:36PM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin 
> 
> More .rodata string saving by avoid __I915__ magic inside WARNs.
> 
> v2: Add parantheses around dev_priv. (Ville Syrjala)
> 
> Signed-off-by: Tvrtko Ursulin 

Reviewed-by: David Weinehall 

Note that once this patch series goes in (or before),
we should have a patch that turns intel_hdmi_to_dev() into
intel_hdmi_to_dev_priv().  If you look at the code in
intel_hdmi.c, almost every (after the dev -> dev_priv transition
I think it's every) instance where it's used converts
dev immediately further to dev_priv.

> ---
>  drivers/gpu/drm/i915/i915_drv.h| 2 +-
>  drivers/gpu/drm/i915/intel_color.c | 6 +++---
>  drivers/gpu/drm/i915/intel_display.c   | 8 
>  drivers/gpu/drm/i915/intel_dp.c| 2 +-
>  drivers/gpu/drm/i915/intel_dsi.c   | 2 +-
>  drivers/gpu/drm/i915/intel_fifo_underrun.c | 2 +-
>  drivers/gpu/drm/i915/intel_hdmi.c  | 5 +++--
>  7 files changed, 14 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 3caa1c767512..1a4698e665be 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2863,7 +2863,7 @@ struct drm_i915_cmd_table {
>  #define HAS_PCH_NOP(dev_priv) (INTEL_PCH_TYPE(dev_priv) == PCH_NOP)
>  #define HAS_PCH_SPLIT(dev_priv) (INTEL_PCH_TYPE(dev_priv) != PCH_NONE)
>  
> -#define HAS_GMCH_DISPLAY(dev) (INTEL_INFO(dev)->has_gmch_display)
> +#define HAS_GMCH_DISPLAY(dev_priv) ((dev_priv)->info.has_gmch_display)
>  
>  /* DPF == dynamic parity feature */
>  #define HAS_L3_DPF(dev) (INTEL_INFO(dev)->has_l3_dpf)
> diff --git a/drivers/gpu/drm/i915/intel_color.c 
> b/drivers/gpu/drm/i915/intel_color.c
> index 95a72771eea6..5362c07932d3 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -273,7 +273,7 @@ static void i9xx_load_luts_internal(struct drm_crtc *crtc,
>   enum pipe pipe = intel_crtc->pipe;
>   int i;
>  
> - if (HAS_GMCH_DISPLAY(dev)) {
> + if (HAS_GMCH_DISPLAY(dev_priv)) {
>   if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI))
>   assert_dsi_pll_enabled(dev_priv);
>   else
> @@ -288,7 +288,7 @@ static void i9xx_load_luts_internal(struct drm_crtc *crtc,
>   (drm_color_lut_extract(lut[i].green, 8) << 8) |
>   drm_color_lut_extract(lut[i].blue, 8);
>  
> - if (HAS_GMCH_DISPLAY(dev))
> + if (HAS_GMCH_DISPLAY(dev_priv))
>   I915_WRITE(PALETTE(pipe, i), word);
>   else
>   I915_WRITE(LGC_PALETTE(pipe, i), word);
> @@ -297,7 +297,7 @@ static void i9xx_load_luts_internal(struct drm_crtc *crtc,
>   for (i = 0; i < 256; i++) {
>   uint32_t word = (i << 16) | (i << 8) | i;
>  
> - if (HAS_GMCH_DISPLAY(dev))
> + if (HAS_GMCH_DISPLAY(dev_priv))
>   I915_WRITE(PALETTE(pipe, i), word);
>   else
>   I915_WRITE(LGC_PALETTE(pipe, i), word);
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 0a69e80821ee..b7685936d324 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5036,7 +5036,7 @@ intel_pre_disable_primary_noatomic(struct drm_crtc 
> *crtc)
>* event which is after the vblank start event, so we need to have a
>* wait-for-vblank between disabling the plane and the pipe.
>*/
> - if (HAS_GMCH_DISPLAY(dev)) {
> + if (HAS_GMCH_DISPLAY(dev_priv)) {
>   intel_set_memory_cxsr(dev_priv, false);
>   dev_priv->wm.vlv.cxsr = false;
>   intel_wait_for_vblank(dev, pipe);
> @@ -5101,7 +5101,7 @@ static void intel_pre_plane_update(struct 
> intel_crtc_state *old_crtc_state)
>   intel_pre_disable_primary(>base);
>   }
>  
> - if (pipe_config->disable_cxsr && HAS_GMCH_DISPLAY(dev)) {
> + if (pipe_config->disable_cxsr && HAS_GMCH_DISPLAY(dev_priv)) {
>   crtc->wm.cxsr_allowed = false;
>  
>   /*
> @@ -10895,7 +10895,7 @@ static void intel_crtc_update_cursor(struct drm_crtc 
> *crtc,
>   pos |= y << CURSOR_Y_SHIFT;
>  
>   /* ILK+ do this automagically */
> - if (HAS_GMCH_DISPLAY(dev) &&
> + if (HAS_GMCH_DISPLAY(dev_priv) &&
>   plane_state->base.rotation == DRM_ROTATE_180) {
>   base += (plane_state->base.crtc_h *
>plane_state->base.crtc_w - 1) * 4;
> @@ -16593,7 +16593,7 @@ static void intel_sanitize_crtc(struct intel_crtc 
> 

[Intel-gfx] [PATCH 03/19] drm/i915: Make HAS_GMCH_DISPLAY only take dev_priv

[Tvrtko Ursulin]

From: Tvrtko Ursulin 

More .rodata string saving by avoid __I915__ magic inside WARNs.

v2: Add parantheses around dev_priv. (Ville Syrjala)

Signed-off-by: Tvrtko Ursulin 
---
 drivers/gpu/drm/i915/i915_drv.h| 2 +-
 drivers/gpu/drm/i915/intel_color.c | 6 +++---
 drivers/gpu/drm/i915/intel_display.c   | 8 
 drivers/gpu/drm/i915/intel_dp.c| 2 +-
 drivers/gpu/drm/i915/intel_dsi.c   | 2 +-
 drivers/gpu/drm/i915/intel_fifo_underrun.c | 2 +-
 drivers/gpu/drm/i915/intel_hdmi.c  | 5 +++--
 7 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3caa1c767512..1a4698e665be 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2863,7 +2863,7 @@ struct drm_i915_cmd_table {
 #define HAS_PCH_NOP(dev_priv) (INTEL_PCH_TYPE(dev_priv) == PCH_NOP)
 #define HAS_PCH_SPLIT(dev_priv) (INTEL_PCH_TYPE(dev_priv) != PCH_NONE)
 
-#define HAS_GMCH_DISPLAY(dev) (INTEL_INFO(dev)->has_gmch_display)
+#define HAS_GMCH_DISPLAY(dev_priv) ((dev_priv)->info.has_gmch_display)
 
 /* DPF == dynamic parity feature */
 #define HAS_L3_DPF(dev) (INTEL_INFO(dev)->has_l3_dpf)
diff --git a/drivers/gpu/drm/i915/intel_color.c 
b/drivers/gpu/drm/i915/intel_color.c
index 95a72771eea6..5362c07932d3 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -273,7 +273,7 @@ static void i9xx_load_luts_internal(struct drm_crtc *crtc,
enum pipe pipe = intel_crtc->pipe;
int i;
 
-   if (HAS_GMCH_DISPLAY(dev)) {
+   if (HAS_GMCH_DISPLAY(dev_priv)) {
if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI))
assert_dsi_pll_enabled(dev_priv);
else
@@ -288,7 +288,7 @@ static void i9xx_load_luts_internal(struct drm_crtc *crtc,
(drm_color_lut_extract(lut[i].green, 8) << 8) |
drm_color_lut_extract(lut[i].blue, 8);
 
-   if (HAS_GMCH_DISPLAY(dev))
+   if (HAS_GMCH_DISPLAY(dev_priv))
I915_WRITE(PALETTE(pipe, i), word);
else
I915_WRITE(LGC_PALETTE(pipe, i), word);
@@ -297,7 +297,7 @@ static void i9xx_load_luts_internal(struct drm_crtc *crtc,
for (i = 0; i < 256; i++) {
uint32_t word = (i << 16) | (i << 8) | i;
 
-   if (HAS_GMCH_DISPLAY(dev))
+   if (HAS_GMCH_DISPLAY(dev_priv))
I915_WRITE(PALETTE(pipe, i), word);
else
I915_WRITE(LGC_PALETTE(pipe, i), word);
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 0a69e80821ee..b7685936d324 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5036,7 +5036,7 @@ intel_pre_disable_primary_noatomic(struct drm_crtc *crtc)
 * event which is after the vblank start event, so we need to have a
 * wait-for-vblank between disabling the plane and the pipe.
 */
-   if (HAS_GMCH_DISPLAY(dev)) {
+   if (HAS_GMCH_DISPLAY(dev_priv)) {
intel_set_memory_cxsr(dev_priv, false);
dev_priv->wm.vlv.cxsr = false;
intel_wait_for_vblank(dev, pipe);
@@ -5101,7 +5101,7 @@ static void intel_pre_plane_update(struct 
intel_crtc_state *old_crtc_state)
intel_pre_disable_primary(>base);
}
 
-   if (pipe_config->disable_cxsr && HAS_GMCH_DISPLAY(dev)) {
+   if (pipe_config->disable_cxsr && HAS_GMCH_DISPLAY(dev_priv)) {
crtc->wm.cxsr_allowed = false;
 
/*
@@ -10895,7 +10895,7 @@ static void intel_crtc_update_cursor(struct drm_crtc 
*crtc,
pos |= y << CURSOR_Y_SHIFT;
 
/* ILK+ do this automagically */
-   if (HAS_GMCH_DISPLAY(dev) &&
+   if (HAS_GMCH_DISPLAY(dev_priv) &&
plane_state->base.rotation == DRM_ROTATE_180) {
base += (plane_state->base.crtc_h *
 plane_state->base.crtc_w - 1) * 4;
@@ -16593,7 +16593,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
if (crtc->active && !intel_crtc_has_encoders(crtc))
intel_crtc_disable_noatomic(>base);
 
-   if (crtc->active || HAS_GMCH_DISPLAY(dev)) {
+   if (crtc->active || HAS_GMCH_DISPLAY(dev_priv)) {
/*
 * We start out with underrun reporting disabled to avoid races.
 * For correct bookkeeping mark this on active crtcs.
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 0b6f1bab671d..51d92a9c6cb1 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++