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

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 8743 bytes --]


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<tvrtko.ursulin@intel.com>
>>
>> More .rodata string saving by avoid __I915__ magic inside WARNs.
>>
>> v2: Add parantheses around dev_priv. (Ville Syrjala)
>>
>> Signed-off-by: Tvrtko Ursulin<tvrtko.ursulin@intel.com>
> Reviewed-by: David Weinehall<david.weinehall@linux.intel.com>
>
> 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(&crtc->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(&crtc->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
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -1583,7 +1583,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>>   				return ret;
>>   		}
>>   
>> -		if (HAS_GMCH_DISPLAY(dev))
>> +		if (HAS_GMCH_DISPLAY(dev_priv))
>>   			intel_gmch_panel_fitting(intel_crtc, pipe_config,
>>   						 intel_connector->panel.fitting_mode);
>>   		else
>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>> index 727adaace96c..5b1e445a80d0 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> @@ -1346,7 +1346,7 @@ static int intel_dsi_set_property(struct drm_connector *connector,
>>   			DRM_DEBUG_KMS("no scaling not supported\n");
>>   			return -EINVAL;
>>   		}
>> -		if (HAS_GMCH_DISPLAY(dev) &&
>> +		if (HAS_GMCH_DISPLAY(to_i915(dev)) &&
>>   		    val == DRM_MODE_SCALE_CENTER) {
>>   			DRM_DEBUG_KMS("centering not supported\n");
>>   			return -EINVAL;
>> diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c
>> index ebb4fed8322e..076893cc3890 100644
>> --- a/drivers/gpu/drm/i915/intel_fifo_underrun.c
>> +++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c
>> @@ -254,7 +254,7 @@ static bool __intel_set_cpu_fifo_underrun_reporting(struct drm_device *dev,
>>   	old = !intel_crtc->cpu_fifo_underrun_disabled;
>>   	intel_crtc->cpu_fifo_underrun_disabled = !enable;
>>   
>> -	if (HAS_GMCH_DISPLAY(dev))
>> +	if (HAS_GMCH_DISPLAY(dev_priv))
>>   		i9xx_set_fifo_underrun_reporting(dev, pipe, enable, old);
>>   	else if (IS_GEN5(dev) || IS_GEN6(dev))
>>   		ironlake_set_fifo_underrun_reporting(dev, pipe, enable);
>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>> index 397e10f4b6f0..ad3ff4fe63cf 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -1265,6 +1265,7 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
>>   {
>>   	struct intel_hdmi *hdmi = intel_attached_hdmi(connector);
>>   	struct drm_device *dev = intel_hdmi_to_dev(hdmi);
>> +	struct drm_i915_private *dev_priv = to_i915(dev);
>>   	enum drm_mode_status status;
>>   	int clock;
>>   	int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
>> @@ -1287,7 +1288,7 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
>>   	status = hdmi_port_clock_valid(hdmi, clock, true);
>>   
>>   	/* if we can't do 8bpc we may still be able to do 12bpc */
>> -	if (!HAS_GMCH_DISPLAY(dev) && status != MODE_OK)
>> +	if (!HAS_GMCH_DISPLAY(dev_priv) && status != MODE_OK)
>>   		status = hdmi_port_clock_valid(hdmi, clock * 3 / 2, true);
>>   
>>   	return status;
>> @@ -1297,7 +1298,7 @@ static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state)
>>   {
>>   	struct drm_device *dev = crtc_state->base.crtc->dev;
>>   
>> -	if (HAS_GMCH_DISPLAY(dev))
>> +	if (HAS_GMCH_DISPLAY(to_i915(dev)))
>>   		return false;
>>   
>>   	/*
>> -- 
>> 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


[-- Attachment #1.2: Type: text/html, Size: 9665 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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