diff mbox series

[v2] drm/i915: use for_each_pipe loop to assign crtc_mask

Message ID 20180907125906.24619-1-mahesh1.kumar@intel.com (mailing list archive)
State New, archived
Headers show
Series [v2] drm/i915: use for_each_pipe loop to assign crtc_mask | expand

Commit Message

Kumar, Mahesh Sept. 7, 2018, 12:59 p.m. UTC
This cleanup patch makes changes to use for_each_pipe loop
during bit-mask assignment of allowed crtc with encoder.

changes:
 - use BIT(i) macro instead of (1 << i) (Chris)

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c  | 4 +++-
 drivers/gpu/drm/i915/intel_dp.c   | 5 ++++-
 drivers/gpu/drm/i915/intel_hdmi.c | 5 ++++-
 3 files changed, 11 insertions(+), 3 deletions(-)

Comments

Jani Nikula Sept. 7, 2018, 1:04 p.m. UTC | #1
On Fri, 07 Sep 2018, Mahesh Kumar <mahesh1.kumar@intel.com> wrote:
> This cleanup patch makes changes to use for_each_pipe loop
> during bit-mask assignment of allowed crtc with encoder.
>
> changes:
>  - use BIT(i) macro instead of (1 << i) (Chris)
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c  | 4 +++-
>  drivers/gpu/drm/i915/intel_dp.c   | 5 ++++-
>  drivers/gpu/drm/i915/intel_hdmi.c | 5 ++++-
>  3 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index cd01a09c5e0f..88dfca245099 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3751,6 +3751,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>  	struct intel_encoder *intel_encoder;
>  	struct drm_encoder *encoder;
>  	bool init_hdmi, init_dp, init_lspcon = false;
> +	u8 i;

Please use int instead of u8 throughout. u8 only makes sense when you
actually need the specific size, or need to be concerned about the size.

BR,
Jani.

>  
>  
>  	init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi ||
> @@ -3801,8 +3802,9 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>  	intel_encoder->type = INTEL_OUTPUT_DDI;
>  	intel_encoder->power_domain = intel_port_to_power_domain(port);
>  	intel_encoder->port = port;
> -	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
>  	intel_encoder->cloneable = 0;
> +	for_each_pipe(dev_priv, i)
> +		intel_encoder->crtc_mask |= BIT(i);
>  
>  	if (INTEL_GEN(dev_priv) >= 11)
>  		intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) &
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 436c22de33b6..1f954debdc55 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -6709,7 +6709,10 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
>  		else
>  			intel_encoder->crtc_mask = (1 << 0) | (1 << 1);
>  	} else {
> -		intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
> +		u8 i;
> +
> +		for_each_pipe(dev_priv, i)
> +			intel_encoder->crtc_mask |= BIT(i);
>  	}
>  	intel_encoder->cloneable = 0;
>  	intel_encoder->port = port;
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index a2dab0b6bde6..647e38de7980 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -2468,7 +2468,10 @@ void intel_hdmi_init(struct drm_i915_private *dev_priv,
>  		else
>  			intel_encoder->crtc_mask = (1 << 0) | (1 << 1);
>  	} else {
> -		intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
> +		u8 i;
> +
> +		for_each_pipe(dev_priv, i)
> +			intel_encoder->crtc_mask |= BIT(i);
>  	}
>  	intel_encoder->cloneable = 1 << INTEL_OUTPUT_ANALOG;
>  	/*
Kumar, Mahesh Sept. 7, 2018, 1:11 p.m. UTC | #2
Hi,

I used u8 because internally num_pipes variable used by for_each_pipe 
macro is of u8 type.
If you think it's good to have int for consistency I can update the patch.

Regards,
-Mahesh

On 9/7/2018 6:34 PM, Jani Nikula wrote:
> On Fri, 07 Sep 2018, Mahesh Kumar <mahesh1.kumar@intel.com> wrote:
>> This cleanup patch makes changes to use for_each_pipe loop
>> during bit-mask assignment of allowed crtc with encoder.
>>
>> changes:
>>   - use BIT(i) macro instead of (1 << i) (Chris)
>>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_ddi.c  | 4 +++-
>>   drivers/gpu/drm/i915/intel_dp.c   | 5 ++++-
>>   drivers/gpu/drm/i915/intel_hdmi.c | 5 ++++-
>>   3 files changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>> index cd01a09c5e0f..88dfca245099 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -3751,6 +3751,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>>   	struct intel_encoder *intel_encoder;
>>   	struct drm_encoder *encoder;
>>   	bool init_hdmi, init_dp, init_lspcon = false;
>> +	u8 i;
> Please use int instead of u8 throughout. u8 only makes sense when you
> actually need the specific size, or need to be concerned about the size.
>
> BR,
> Jani.
>
>>   
>>   
>>   	init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi ||
>> @@ -3801,8 +3802,9 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>>   	intel_encoder->type = INTEL_OUTPUT_DDI;
>>   	intel_encoder->power_domain = intel_port_to_power_domain(port);
>>   	intel_encoder->port = port;
>> -	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
>>   	intel_encoder->cloneable = 0;
>> +	for_each_pipe(dev_priv, i)
>> +		intel_encoder->crtc_mask |= BIT(i);
>>   
>>   	if (INTEL_GEN(dev_priv) >= 11)
>>   		intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) &
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 436c22de33b6..1f954debdc55 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -6709,7 +6709,10 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
>>   		else
>>   			intel_encoder->crtc_mask = (1 << 0) | (1 << 1);
>>   	} else {
>> -		intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
>> +		u8 i;
>> +
>> +		for_each_pipe(dev_priv, i)
>> +			intel_encoder->crtc_mask |= BIT(i);
>>   	}
>>   	intel_encoder->cloneable = 0;
>>   	intel_encoder->port = port;
>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>> index a2dab0b6bde6..647e38de7980 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -2468,7 +2468,10 @@ void intel_hdmi_init(struct drm_i915_private *dev_priv,
>>   		else
>>   			intel_encoder->crtc_mask = (1 << 0) | (1 << 1);
>>   	} else {
>> -		intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
>> +		u8 i;
>> +
>> +		for_each_pipe(dev_priv, i)
>> +			intel_encoder->crtc_mask |= BIT(i);
>>   	}
>>   	intel_encoder->cloneable = 1 << INTEL_OUTPUT_ANALOG;
>>   	/*
Ville Syrjälä Sept. 7, 2018, 2:49 p.m. UTC | #3
On Fri, Sep 07, 2018 at 04:04:03PM +0300, Jani Nikula wrote:
> On Fri, 07 Sep 2018, Mahesh Kumar <mahesh1.kumar@intel.com> wrote:
> > This cleanup patch makes changes to use for_each_pipe loop
> > during bit-mask assignment of allowed crtc with encoder.
> >
> > changes:
> >  - use BIT(i) macro instead of (1 << i) (Chris)
> >
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c  | 4 +++-
> >  drivers/gpu/drm/i915/intel_dp.c   | 5 ++++-
> >  drivers/gpu/drm/i915/intel_hdmi.c | 5 ++++-
> >  3 files changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index cd01a09c5e0f..88dfca245099 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -3751,6 +3751,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> >  	struct intel_encoder *intel_encoder;
> >  	struct drm_encoder *encoder;
> >  	bool init_hdmi, init_dp, init_lspcon = false;
> > +	u8 i;
> 
> Please use int instead of u8 throughout. u8 only makes sense when you
> actually need the specific size, or need to be concerned about the size.

Actually please use 'enum pipe pipe'.

> 
> BR,
> Jani.
> 
> >  
> >  
> >  	init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi ||
> > @@ -3801,8 +3802,9 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> >  	intel_encoder->type = INTEL_OUTPUT_DDI;
> >  	intel_encoder->power_domain = intel_port_to_power_domain(port);
> >  	intel_encoder->port = port;
> > -	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
> >  	intel_encoder->cloneable = 0;
> > +	for_each_pipe(dev_priv, i)
> > +		intel_encoder->crtc_mask |= BIT(i);
> >  
> >  	if (INTEL_GEN(dev_priv) >= 11)
> >  		intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) &
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 436c22de33b6..1f954debdc55 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -6709,7 +6709,10 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
> >  		else
> >  			intel_encoder->crtc_mask = (1 << 0) | (1 << 1);
> >  	} else {
> > -		intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
> > +		u8 i;
> > +
> > +		for_each_pipe(dev_priv, i)
> > +			intel_encoder->crtc_mask |= BIT(i);
> >  	}
> >  	intel_encoder->cloneable = 0;
> >  	intel_encoder->port = port;
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > index a2dab0b6bde6..647e38de7980 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -2468,7 +2468,10 @@ void intel_hdmi_init(struct drm_i915_private *dev_priv,
> >  		else
> >  			intel_encoder->crtc_mask = (1 << 0) | (1 << 1);
> >  	} else {
> > -		intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
> > +		u8 i;
> > +
> > +		for_each_pipe(dev_priv, i)
> > +			intel_encoder->crtc_mask |= BIT(i);
> >  	}
> >  	intel_encoder->cloneable = 1 << INTEL_OUTPUT_ANALOG;
> >  	/*
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Jani Nikula Sept. 10, 2018, 9:04 a.m. UTC | #4
On Fri, 07 Sep 2018, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Fri, Sep 07, 2018 at 04:04:03PM +0300, Jani Nikula wrote:
>> On Fri, 07 Sep 2018, Mahesh Kumar <mahesh1.kumar@intel.com> wrote:
>> > This cleanup patch makes changes to use for_each_pipe loop
>> > during bit-mask assignment of allowed crtc with encoder.
>> >
>> > changes:
>> >  - use BIT(i) macro instead of (1 << i) (Chris)
>> >
>> > Cc: Jani Nikula <jani.nikula@intel.com>
>> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> > Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/intel_ddi.c  | 4 +++-
>> >  drivers/gpu/drm/i915/intel_dp.c   | 5 ++++-
>> >  drivers/gpu/drm/i915/intel_hdmi.c | 5 ++++-
>> >  3 files changed, 11 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>> > index cd01a09c5e0f..88dfca245099 100644
>> > --- a/drivers/gpu/drm/i915/intel_ddi.c
>> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> > @@ -3751,6 +3751,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>> >  	struct intel_encoder *intel_encoder;
>> >  	struct drm_encoder *encoder;
>> >  	bool init_hdmi, init_dp, init_lspcon = false;
>> > +	u8 i;
>> 
>> Please use int instead of u8 throughout. u8 only makes sense when you
>> actually need the specific size, or need to be concerned about the size.
>
> Actually please use 'enum pipe pipe'.

D'oh, of course.

BR,
Jani.


>
>> 
>> BR,
>> Jani.
>> 
>> >  
>> >  
>> >  	init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi ||
>> > @@ -3801,8 +3802,9 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>> >  	intel_encoder->type = INTEL_OUTPUT_DDI;
>> >  	intel_encoder->power_domain = intel_port_to_power_domain(port);
>> >  	intel_encoder->port = port;
>> > -	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
>> >  	intel_encoder->cloneable = 0;
>> > +	for_each_pipe(dev_priv, i)
>> > +		intel_encoder->crtc_mask |= BIT(i);
>> >  
>> >  	if (INTEL_GEN(dev_priv) >= 11)
>> >  		intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) &
>> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> > index 436c22de33b6..1f954debdc55 100644
>> > --- a/drivers/gpu/drm/i915/intel_dp.c
>> > +++ b/drivers/gpu/drm/i915/intel_dp.c
>> > @@ -6709,7 +6709,10 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
>> >  		else
>> >  			intel_encoder->crtc_mask = (1 << 0) | (1 << 1);
>> >  	} else {
>> > -		intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
>> > +		u8 i;
>> > +
>> > +		for_each_pipe(dev_priv, i)
>> > +			intel_encoder->crtc_mask |= BIT(i);
>> >  	}
>> >  	intel_encoder->cloneable = 0;
>> >  	intel_encoder->port = port;
>> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>> > index a2dab0b6bde6..647e38de7980 100644
>> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> > @@ -2468,7 +2468,10 @@ void intel_hdmi_init(struct drm_i915_private *dev_priv,
>> >  		else
>> >  			intel_encoder->crtc_mask = (1 << 0) | (1 << 1);
>> >  	} else {
>> > -		intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
>> > +		u8 i;
>> > +
>> > +		for_each_pipe(dev_priv, i)
>> > +			intel_encoder->crtc_mask |= BIT(i);
>> >  	}
>> >  	intel_encoder->cloneable = 1 << INTEL_OUTPUT_ANALOG;
>> >  	/*
>> 
>> -- 
>> Jani Nikula, Intel Open Source Graphics Center
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index cd01a09c5e0f..88dfca245099 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3751,6 +3751,7 @@  void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 	struct intel_encoder *intel_encoder;
 	struct drm_encoder *encoder;
 	bool init_hdmi, init_dp, init_lspcon = false;
+	u8 i;
 
 
 	init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi ||
@@ -3801,8 +3802,9 @@  void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 	intel_encoder->type = INTEL_OUTPUT_DDI;
 	intel_encoder->power_domain = intel_port_to_power_domain(port);
 	intel_encoder->port = port;
-	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
 	intel_encoder->cloneable = 0;
+	for_each_pipe(dev_priv, i)
+		intel_encoder->crtc_mask |= BIT(i);
 
 	if (INTEL_GEN(dev_priv) >= 11)
 		intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) &
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 436c22de33b6..1f954debdc55 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -6709,7 +6709,10 @@  bool intel_dp_init(struct drm_i915_private *dev_priv,
 		else
 			intel_encoder->crtc_mask = (1 << 0) | (1 << 1);
 	} else {
-		intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
+		u8 i;
+
+		for_each_pipe(dev_priv, i)
+			intel_encoder->crtc_mask |= BIT(i);
 	}
 	intel_encoder->cloneable = 0;
 	intel_encoder->port = port;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index a2dab0b6bde6..647e38de7980 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -2468,7 +2468,10 @@  void intel_hdmi_init(struct drm_i915_private *dev_priv,
 		else
 			intel_encoder->crtc_mask = (1 << 0) | (1 << 1);
 	} else {
-		intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
+		u8 i;
+
+		for_each_pipe(dev_priv, i)
+			intel_encoder->crtc_mask |= BIT(i);
 	}
 	intel_encoder->cloneable = 1 << INTEL_OUTPUT_ANALOG;
 	/*