[PATCH v2 1/3] i915/dp: add intel_dp_tps3_supported helper

[PATCH v2 1/3] i915/dp: add intel_dp_tps3_supported helper

[Jani Nikula]

We can add this to drm dp helpers later.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 9e90a2be22fa..12096c1df622 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1222,6 +1222,13 @@ intel_dp_sink_rates(struct intel_dp *intel_dp, const int **sink_rates)
 	return (intel_dp_max_link_bw(intel_dp) >> 3) + 1;
 }
 
+static inline bool
+intel_dp_tps3_supported(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
+{
+	return dpcd[DP_DPCD_REV] >= 0x12 &&
+		dpcd[DP_MAX_LANE_COUNT] & DP_TPS3_SUPPORTED;
+}
+
 static bool intel_dp_source_supports_hbr2(struct drm_device *dev)
 {
 	/* WaDisableHBR2:skl */
-- 
2.1.4

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

[PATCH v2 2/3] drm/i915/dp: use the drm dp helper for determining sink tps3 support

[Jani Nikula]

No functional changes.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 12096c1df622..50ba527763e9 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4042,8 +4042,7 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 	 * SKL < B0: due it's WaDisableHBR2 is the only exception where TP3 is
 	 * supported but still not enabled.
 	 */
-	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x12 &&
-	    intel_dp->dpcd[DP_MAX_LANE_COUNT] & DP_TPS3_SUPPORTED &&
+	if (intel_dp_tps3_supported(intel_dp->dpcd) &&
 	    intel_dp_source_supports_hbr2(dev)) {
 		intel_dp->use_tps3 = true;
 		DRM_DEBUG_KMS("Displayport TPS3 supported\n");
-- 
2.1.4

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

Re: [PATCH v2 2/3] drm/i915/dp: use the drm dp helper for determining sink tps3 support

[Daniel Vetter]

On Thu, Aug 27, 2015 at 01:25:37PM +0300, Jani Nikula wrote:
> No functional changes.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Merged the first 2 patches to dinq (rebased on top of the drm dp helper as
the series originally did). But can't merge more since this depends upon
the tp3 reverts in dinf, so need a suitable backmerge first.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/3] drm/i915/dp: use the drm dp helper for determining sink tps3 support

[Jani Nikula]

On Tue, 01 Sep 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Aug 27, 2015 at 01:25:37PM +0300, Jani Nikula wrote:
>> No functional changes.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> Merged the first 2 patches to dinq (rebased on top of the drm dp helper as
> the series originally did). But can't merge more since this depends upon
> the tp3 reverts in dinf, so need a suitable backmerge first.

Okay, when you get to it, I've sent the remaining patches rebased on top
of nightly [1].

BR,
Jani.


[1] http://mid.gmane.org/cover.1441267909.git.jani.nikula@intel.com


-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 3/3] drm/i915/dp: move TPS3 logic to where it's used

[Jani Nikula]

There is no need to have a separate flag for tps3 as the information is
only used at one location. Move the logic there to make it easier to
follow.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c  | 31 +++++++++++++++++--------------
 drivers/gpu/drm/i915/intel_drv.h |  1 -
 2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 50ba527763e9..12bce36065a1 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3848,13 +3848,25 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
 void
 intel_dp_complete_link_train(struct intel_dp *intel_dp)
 {
+	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
+	struct drm_device *dev = dig_port->base.base.dev;
 	bool channel_eq = false;
 	int tries, cr_tries;
 	uint32_t DP = intel_dp->DP;
 	uint32_t training_pattern = DP_TRAINING_PATTERN_2;
 
-	/* Training Pattern 3 for HBR2 or 1.2 devices that support it*/
-	if (intel_dp->link_rate == 540000 || intel_dp->use_tps3)
+	/*
+	 * Training Pattern 3 for HBR2 or 1.2 devices that support it.
+	 *
+	 * Intel platforms that support HBR2 also support TPS3. TPS3 support is
+	 * also mandatory for downstream devices that support HBR2.
+	 *
+	 * Due to WaDisableHBR2 SKL < B0 is the only exception where TPS3 is
+	 * supported but still not enabled.
+	 */
+	if (intel_dp->link_rate == 540000 ||
+	    (intel_dp_source_supports_hbr2(dev) &&
+	     intel_dp_tps3_supported(intel_dp->dpcd)))
 		training_pattern = DP_TRAINING_PATTERN_3;
 
 	/* channel equalization */
@@ -4036,18 +4048,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 		}
 	}
 
-	/* Training Pattern 3 support, Intel platforms that support HBR2 alone
-	 * have support for TP3 hence that check is used along with dpcd check
-	 * to ensure TP3 can be enabled.
-	 * SKL < B0: due it's WaDisableHBR2 is the only exception where TP3 is
-	 * supported but still not enabled.
-	 */
-	if (intel_dp_tps3_supported(intel_dp->dpcd) &&
-	    intel_dp_source_supports_hbr2(dev)) {
-		intel_dp->use_tps3 = true;
-		DRM_DEBUG_KMS("Displayport TPS3 supported\n");
-	} else
-		intel_dp->use_tps3 = false;
+	DRM_DEBUG_KMS("Display Port TPS3 support: source %s, sink %s\n",
+		      intel_dp_source_supports_hbr2(dev) ? "yes" : "no",
+		      intel_dp_tps3_supported(intel_dp->dpcd) ? "yes" : "no");
 
 	/* Intermediate frequency support */
 	if (is_edp(intel_dp) &&
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c61ba47eda7c..fa9840fbccba 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -744,7 +744,6 @@ struct intel_dp {
 	enum pipe pps_pipe;
 	struct edp_power_seq pps_delays;
 
-	bool use_tps3;
 	bool can_mst; /* this port supports mst */
 	bool is_mst;
 	int active_mst_links;
-- 
2.1.4

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

Re: [PATCH v2 3/3] drm/i915/dp: move TPS3 logic to where it's used

[Ville Syrjälä]

On Thu, Aug 27, 2015 at 01:25:38PM +0300, Jani Nikula wrote:
> There is no need to have a separate flag for tps3 as the information is
> only used at one location. Move the logic there to make it easier to
> follow.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 31 +++++++++++++++++--------------
>  drivers/gpu/drm/i915/intel_drv.h |  1 -
>  2 files changed, 17 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 50ba527763e9..12bce36065a1 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3848,13 +3848,25 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
>  void
>  intel_dp_complete_link_train(struct intel_dp *intel_dp)
>  {
> +	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
> +	struct drm_device *dev = dig_port->base.base.dev;
>  	bool channel_eq = false;
>  	int tries, cr_tries;
>  	uint32_t DP = intel_dp->DP;
>  	uint32_t training_pattern = DP_TRAINING_PATTERN_2;
>  
> -	/* Training Pattern 3 for HBR2 or 1.2 devices that support it*/
> -	if (intel_dp->link_rate == 540000 || intel_dp->use_tps3)
> +	/*
> +	 * Training Pattern 3 for HBR2 or 1.2 devices that support it.
> +	 *
> +	 * Intel platforms that support HBR2 also support TPS3. TPS3 support is
> +	 * also mandatory for downstream devices that support HBR2.
> +	 *
> +	 * Due to WaDisableHBR2 SKL < B0 is the only exception where TPS3 is
> +	 * supported but still not enabled.
> +	 */
> +	if (intel_dp->link_rate == 540000 ||
> +	    (intel_dp_source_supports_hbr2(dev) &&
> +	     intel_dp_tps3_supported(intel_dp->dpcd)))

I'm thinking we could just kill the link_rate check here. It would
only make a difference if the sink lied in its DPCD.

>  		training_pattern = DP_TRAINING_PATTERN_3;
>  
>  	/* channel equalization */
> @@ -4036,18 +4048,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
>  		}
>  	}
>  
> -	/* Training Pattern 3 support, Intel platforms that support HBR2 alone
> -	 * have support for TP3 hence that check is used along with dpcd check
> -	 * to ensure TP3 can be enabled.
> -	 * SKL < B0: due it's WaDisableHBR2 is the only exception where TP3 is
> -	 * supported but still not enabled.
> -	 */
> -	if (intel_dp_tps3_supported(intel_dp->dpcd) &&
> -	    intel_dp_source_supports_hbr2(dev)) {
> -		intel_dp->use_tps3 = true;
> -		DRM_DEBUG_KMS("Displayport TPS3 supported\n");
> -	} else
> -		intel_dp->use_tps3 = false;
> +	DRM_DEBUG_KMS("Display Port TPS3 support: source %s, sink %s\n",
> +		      intel_dp_source_supports_hbr2(dev) ? "yes" : "no",
> +		      intel_dp_tps3_supported(intel_dp->dpcd) ? "yes" : "no");

I guess we don't have a one true yesno() macro. I think we have such
things in specific files. Maybe make it available everywhere?

But anyway, those are just food for thought, and the series looks good
so
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  
>  	/* Intermediate frequency support */
>  	if (is_edp(intel_dp) &&
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index c61ba47eda7c..fa9840fbccba 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -744,7 +744,6 @@ struct intel_dp {
>  	enum pipe pps_pipe;
>  	struct edp_power_seq pps_delays;
>  
> -	bool use_tps3;
>  	bool can_mst; /* this port supports mst */
>  	bool is_mst;
>  	int active_mst_links;
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[PATCH 0/3] follow-up

[Jani Nikula]

These three patches address Ville's review comments on top of the series.

BR,
Jani.


Jani Nikula (3):
  drm/i915: ignore link rate in TPS3 selection
  drm/i915: add yesno utility function
  drm/i915: use the yesno helper for logging

 drivers/gpu/drm/i915/i915_debugfs.c   | 22 +++++++---------------
 drivers/gpu/drm/i915/i915_drv.h       |  5 +++++
 drivers/gpu/drm/i915/i915_gpu_error.c |  5 -----
 drivers/gpu/drm/i915/intel_dp.c       | 11 ++++++-----
 drivers/gpu/drm/i915/intel_pm.c       |  4 ++--
 5 files changed, 20 insertions(+), 27 deletions(-)

-- 
2.1.4

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

[PATCH 1/3] drm/i915: ignore link rate in TPS3 selection

[Jani Nikula]

TPS3 is mandatory for downstream devices that support HBR2, and Intel
platforms that support HBR2 also support TPS3. Whenever TPS3 is
supported by both the source and sink, it should be used. In other
words, whenever the source and sink are capable of 5.4 Gbps link, we
should anyway go for TPS3, regardless of the link rate being selected.

Log an error if the sink has advertized HBR2 capability without TPS3
capability.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 12bce36065a1..4e68681465d2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3864,10 +3864,11 @@ intel_dp_complete_link_train(struct intel_dp *intel_dp)
 	 * Due to WaDisableHBR2 SKL < B0 is the only exception where TPS3 is
 	 * supported but still not enabled.
 	 */
-	if (intel_dp->link_rate == 540000 ||
-	    (intel_dp_source_supports_hbr2(dev) &&
-	     intel_dp_tps3_supported(intel_dp->dpcd)))
+	if (intel_dp_source_supports_hbr2(dev) &&
+	    intel_dp_tps3_supported(intel_dp->dpcd))
 		training_pattern = DP_TRAINING_PATTERN_3;
+	else if (intel_dp->link_rate == 540000)
+		DRM_ERROR("5.4 Gbps link rate without HBR2/TPS3 support\n");
 
 	/* channel equalization */
 	if (!intel_dp_set_link_train(intel_dp, &DP,
-- 
2.1.4

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

[PATCH 2/3] drm/i915: add yesno utility function

[Jani Nikula]

Add a common function to return "yes" or "no" string based on the
argument, and drop the local versions of it.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c   | 5 -----
 drivers/gpu/drm/i915/i915_drv.h       | 5 +++++
 drivers/gpu/drm/i915/i915_gpu_error.c | 5 -----
 3 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 4088cb1b95d9..de7ae1c398ec 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -46,11 +46,6 @@ enum {
 	PINNED_LIST,
 };
 
-static const char *yesno(int v)
-{
-	return v ? "yes" : "no";
-}
-
 /* As the drm_debugfs_init() routines are called before dev->dev_private is
  * allocated we need to hook into the minor for release. */
 static int
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8c938451a05e..118223a3e155 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -106,6 +106,11 @@
 	unlikely(__ret_warn_on);					\
 })
 
+static inline const char *yesno(bool v)
+{
+	return v ? "yes" : "no";
+}
+
 enum pipe {
 	INVALID_PIPE = -1,
 	PIPE_A = 0,
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 493e9b294a69..3379f9c1ef88 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -30,11 +30,6 @@
 #include <generated/utsrelease.h>
 #include "i915_drv.h"
 
-static const char *yesno(int v)
-{
-	return v ? "yes" : "no";
-}
-
 static const char *ring_str(int ring)
 {
 	switch (ring) {
-- 
2.1.4

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

Re: [PATCH 2/3] drm/i915: add yesno utility function

[Chris Wilson]

On Thu, Aug 27, 2015 at 04:23:30PM +0300, Jani Nikula wrote:
> Add a common function to return "yes" or "no" string based on the
> argument, and drop the local versions of it.

Purely out of curiosity, gcc is able to amalgamate the constant strings
(I remember reading that it is intelligent enough to do so), right? i.e.
size i915.ko doesn't change (at least .data, we may see .text
differences for gcc having different ideas about inlines)?
-Chris

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

Re: [PATCH 2/3] drm/i915: add yesno utility function

[Jani Nikula]

On Thu, 27 Aug 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Thu, Aug 27, 2015 at 04:23:30PM +0300, Jani Nikula wrote:
>> Add a common function to return "yes" or "no" string based on the
>> argument, and drop the local versions of it.
>
> Purely out of curiosity, gcc is able to amalgamate the constant strings
> (I remember reading that it is intelligent enough to do so), right? i.e.
> size i915.ko doesn't change (at least .data, we may see .text
> differences for gcc having different ideas about inlines)?

I admit to giving GCC the benefit of the doubt. I may be naïve that way,
trusting the tools to do what seems like the obviously right thing to
do.

If GCC lets us down, we could try something like the yesno version in
drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c. GCC not doing the
right thing with that would be violating the standard.

BR,
Jani.

> -Chris
>
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915: add yesno utility function

[Jani Nikula]

On Thu, 27 Aug 2015, Jani Nikula <jani.nikula@intel.com> wrote:
> On Thu, 27 Aug 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> On Thu, Aug 27, 2015 at 04:23:30PM +0300, Jani Nikula wrote:
>>> Add a common function to return "yes" or "no" string based on the
>>> argument, and drop the local versions of it.
>>
>> Purely out of curiosity, gcc is able to amalgamate the constant strings
>> (I remember reading that it is intelligent enough to do so), right? i.e.
>> size i915.ko doesn't change (at least .data, we may see .text
>> differences for gcc having different ideas about inlines)?
>
> I admit to giving GCC the benefit of the doubt. I may be naïve that way,
> trusting the tools to do what seems like the obviously right thing to
> do.
>
> If GCC lets us down, we could try something like the yesno version in
> drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c. GCC not doing the
> right thing with that would be violating the standard.

AFAICT GCC does the right thing with the patch.

BR,
Jani.


>
> BR,
> Jani.
>
>> -Chris
>>
>> -- 
>> Chris Wilson, Intel Open Source Technology Centre
>
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915: add yesno utility function

[Chris Wilson]

On Mon, Aug 31, 2015 at 05:23:27PM +0300, Jani Nikula wrote:
> On Thu, 27 Aug 2015, Jani Nikula <jani.nikula@intel.com> wrote:
> > On Thu, 27 Aug 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >> On Thu, Aug 27, 2015 at 04:23:30PM +0300, Jani Nikula wrote:
> >>> Add a common function to return "yes" or "no" string based on the
> >>> argument, and drop the local versions of it.
> >>
> >> Purely out of curiosity, gcc is able to amalgamate the constant strings
> >> (I remember reading that it is intelligent enough to do so), right? i.e.
> >> size i915.ko doesn't change (at least .data, we may see .text
> >> differences for gcc having different ideas about inlines)?
> >
> > I admit to giving GCC the benefit of the doubt. I may be naïve that way,
> > trusting the tools to do what seems like the obviously right thing to
> > do.
> >
> > If GCC lets us down, we could try something like the yesno version in
> > drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c. GCC not doing the
> > right thing with that would be violating the standard.
> 
> AFAICT GCC does the right thing with the patch.

Fwiw, I didn't see any harm in the series, so
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

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

Re: [PATCH 2/3] drm/i915: add yesno utility function

[Daniel Vetter]

On Mon, Aug 31, 2015 at 03:33:13PM +0100, Chris Wilson wrote:
> On Mon, Aug 31, 2015 at 05:23:27PM +0300, Jani Nikula wrote:
> > On Thu, 27 Aug 2015, Jani Nikula <jani.nikula@intel.com> wrote:
> > > On Thu, 27 Aug 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > >> On Thu, Aug 27, 2015 at 04:23:30PM +0300, Jani Nikula wrote:
> > >>> Add a common function to return "yes" or "no" string based on the
> > >>> argument, and drop the local versions of it.
> > >>
> > >> Purely out of curiosity, gcc is able to amalgamate the constant strings
> > >> (I remember reading that it is intelligent enough to do so), right? i.e.
> > >> size i915.ko doesn't change (at least .data, we may see .text
> > >> differences for gcc having different ideas about inlines)?
> > >
> > > I admit to giving GCC the benefit of the doubt. I may be naïve that way,
> > > trusting the tools to do what seems like the obviously right thing to
> > > do.
> > >
> > > If GCC lets us down, we could try something like the yesno version in
> > > drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c. GCC not doing the
> > > right thing with that would be violating the standard.
> > 
> > AFAICT GCC does the right thing with the patch.
> 
> Fwiw, I didn't see any harm in the series, so
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Merged just this patch (due to conflict fun for now) to dinq, thanks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] drm/i915: use the yesno helper for logging

[Jani Nikula]

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 17 +++++++----------
 drivers/gpu/drm/i915/intel_dp.c     |  4 ++--
 drivers/gpu/drm/i915/intel_pm.c     |  4 ++--
 3 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index de7ae1c398ec..df24ac399416 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1382,17 +1382,16 @@ static int ironlake_drpc_info(struct seq_file *m)
 	intel_runtime_pm_put(dev_priv);
 	mutex_unlock(&dev->struct_mutex);
 
-	seq_printf(m, "HD boost: %s\n", (rgvmodectl & MEMMODE_BOOST_EN) ?
-		   "yes" : "no");
+	seq_printf(m, "HD boost: %s\n", yesno(rgvmodectl & MEMMODE_BOOST_EN));
 	seq_printf(m, "Boost freq: %d\n",
 		   (rgvmodectl & MEMMODE_BOOST_FREQ_MASK) >>
 		   MEMMODE_BOOST_FREQ_SHIFT);
 	seq_printf(m, "HW control enabled: %s\n",
-		   rgvmodectl & MEMMODE_HWIDLE_EN ? "yes" : "no");
+		   yesno(rgvmodectl & MEMMODE_HWIDLE_EN));
 	seq_printf(m, "SW control enabled: %s\n",
-		   rgvmodectl & MEMMODE_SWMODE_EN ? "yes" : "no");
+		   yesno(rgvmodectl & MEMMODE_SWMODE_EN));
 	seq_printf(m, "Gated voltage change: %s\n",
-		   rgvmodectl & MEMMODE_RCLK_GATE ? "yes" : "no");
+		   yesno(rgvmodectl & MEMMODE_RCLK_GATE));
 	seq_printf(m, "Starting frequency: P%d\n",
 		   (rgvmodectl & MEMMODE_FSTART_MASK) >> MEMMODE_FSTART_SHIFT);
 	seq_printf(m, "Max P-state: P%d\n",
@@ -1401,7 +1400,7 @@ static int ironlake_drpc_info(struct seq_file *m)
 	seq_printf(m, "RS1 VID: %d\n", (crstandvid & 0x3f));
 	seq_printf(m, "RS2 VID: %d\n", ((crstandvid >> 8) & 0x3f));
 	seq_printf(m, "Render standby enabled: %s\n",
-		   (rstdbyctl & RCX_SW_EXIT) ? "no" : "yes");
+		   yesno(!(rstdbyctl & RCX_SW_EXIT)));
 	seq_puts(m, "Current RS state: ");
 	switch (rstdbyctl & RSX_STATUS_MASK) {
 	case RSX_STATUS_ON:
@@ -2842,8 +2841,7 @@ static void intel_dp_info(struct seq_file *m,
 	struct intel_dp *intel_dp = enc_to_intel_dp(&intel_encoder->base);
 
 	seq_printf(m, "\tDPCD rev: %x\n", intel_dp->dpcd[DP_DPCD_REV]);
-	seq_printf(m, "\taudio support: %s\n", intel_dp->has_audio ? "yes" :
-		   "no");
+	seq_printf(m, "\taudio support: %s\n", yesno(intel_dp->has_audio));
 	if (intel_encoder->type == INTEL_OUTPUT_EDP)
 		intel_panel_info(m, &intel_connector->panel);
 }
@@ -2854,8 +2852,7 @@ static void intel_hdmi_info(struct seq_file *m,
 	struct intel_encoder *intel_encoder = intel_connector->encoder;
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&intel_encoder->base);
 
-	seq_printf(m, "\taudio support: %s\n", intel_hdmi->has_audio ? "yes" :
-		   "no");
+	seq_printf(m, "\taudio support: %s\n", yesno(intel_hdmi->has_audio));
 }
 
 static void intel_lvds_info(struct seq_file *m,
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 4e68681465d2..b55c136cd786 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4050,8 +4050,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp)
 	}
 
 	DRM_DEBUG_KMS("Display Port TPS3 support: source %s, sink %s\n",
-		      intel_dp_source_supports_hbr2(dev) ? "yes" : "no",
-		      intel_dp_tps3_supported(intel_dp->dpcd) ? "yes" : "no");
+		      yesno(intel_dp_source_supports_hbr2(dev)),
+		      yesno(intel_dp_tps3_supported(intel_dp->dpcd)));
 
 	/* Intermediate frequency support */
 	if (is_edp(intel_dp) &&
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index fff0c22682ee..2a463ed3071a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5549,7 +5549,7 @@ static void cherryview_enable_rps(struct drm_device *dev)
 	/* RPS code assumes GPLL is used */
 	WARN_ONCE((val & GPLLENABLE) == 0, "GPLL not enabled\n");
 
-	DRM_DEBUG_DRIVER("GPLL enabled? %s\n", val & GPLLENABLE ? "yes" : "no");
+	DRM_DEBUG_DRIVER("GPLL enabled? %s\n", yesno(val & GPLLENABLE));
 	DRM_DEBUG_DRIVER("GPU status: 0x%08x\n", val);
 
 	dev_priv->rps.cur_freq = (val >> 8) & 0xff;
@@ -5639,7 +5639,7 @@ static void valleyview_enable_rps(struct drm_device *dev)
 	/* RPS code assumes GPLL is used */
 	WARN_ONCE((val & GPLLENABLE) == 0, "GPLL not enabled\n");
 
-	DRM_DEBUG_DRIVER("GPLL enabled? %s\n", val & GPLLENABLE ? "yes" : "no");
+	DRM_DEBUG_DRIVER("GPLL enabled? %s\n", yesno(val & GPLLENABLE));
 	DRM_DEBUG_DRIVER("GPU status: 0x%08x\n", val);
 
 	dev_priv->rps.cur_freq = (val >> 8) & 0xff;
-- 
2.1.4

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

Re: [PATCH v2 3/3] drm/i915/dp: move TPS3 logic to where it's used

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 7269
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
ILK                                  253/253              253/253
SNB                                  248/248              248/248
IVB                                  281/281              281/281
BYT                 -2              234/234              232/234
HSW                                  317/317              317/317
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*BYT  igt@gem_partial_pwrite_pread@reads      PASS(1)      FAIL(1)
*BYT  igt@gem_partial_pwrite_pread@reads-uncached      PASS(1)      FAIL(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx