* [PATCH v2 1/3] i915/dp: add intel_dp_tps3_supported helper @ 2015-08-27 10:25 Jani Nikula 2015-08-27 10:25 ` [PATCH v2 2/3] drm/i915/dp: use the drm dp helper for determining sink tps3 support Jani Nikula 2015-08-27 10:25 ` [PATCH v2 3/3] drm/i915/dp: move TPS3 logic to where it's used Jani Nikula 0 siblings, 2 replies; 16+ messages in thread From: Jani Nikula @ 2015-08-27 10:25 UTC (permalink / raw) To: intel-gfx; +Cc: 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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 2/3] drm/i915/dp: use the drm dp helper for determining sink tps3 support 2015-08-27 10:25 [PATCH v2 1/3] i915/dp: add intel_dp_tps3_supported helper Jani Nikula @ 2015-08-27 10:25 ` Jani Nikula 2015-09-01 9:37 ` Daniel Vetter 2015-08-27 10:25 ` [PATCH v2 3/3] drm/i915/dp: move TPS3 logic to where it's used Jani Nikula 1 sibling, 1 reply; 16+ messages in thread From: Jani Nikula @ 2015-08-27 10:25 UTC (permalink / raw) To: intel-gfx; +Cc: 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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/3] drm/i915/dp: use the drm dp helper for determining sink tps3 support 2015-08-27 10:25 ` [PATCH v2 2/3] drm/i915/dp: use the drm dp helper for determining sink tps3 support Jani Nikula @ 2015-09-01 9:37 ` Daniel Vetter 2015-09-03 8:20 ` Jani Nikula 0 siblings, 1 reply; 16+ messages in thread From: Daniel Vetter @ 2015-09-01 9:37 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/3] drm/i915/dp: use the drm dp helper for determining sink tps3 support 2015-09-01 9:37 ` Daniel Vetter @ 2015-09-03 8:20 ` Jani Nikula 0 siblings, 0 replies; 16+ messages in thread From: Jani Nikula @ 2015-09-03 8:20 UTC (permalink / raw) To: Daniel Vetter; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 3/3] drm/i915/dp: move TPS3 logic to where it's used 2015-08-27 10:25 [PATCH v2 1/3] i915/dp: add intel_dp_tps3_supported helper Jani Nikula 2015-08-27 10:25 ` [PATCH v2 2/3] drm/i915/dp: use the drm dp helper for determining sink tps3 support Jani Nikula @ 2015-08-27 10:25 ` Jani Nikula 2015-08-27 12:15 ` Ville Syrjälä 2015-08-29 19:00 ` [PATCH v2 3/3] drm/i915/dp: move TPS3 logic to where it's used shuang.he 1 sibling, 2 replies; 16+ messages in thread From: Jani Nikula @ 2015-08-27 10:25 UTC (permalink / raw) To: intel-gfx; +Cc: 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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 3/3] drm/i915/dp: move TPS3 logic to where it's used 2015-08-27 10:25 ` [PATCH v2 3/3] drm/i915/dp: move TPS3 logic to where it's used Jani Nikula @ 2015-08-27 12:15 ` Ville Syrjälä 2015-08-27 13:23 ` [PATCH 0/3] follow-up Jani Nikula 2015-08-29 19:00 ` [PATCH v2 3/3] drm/i915/dp: move TPS3 logic to where it's used shuang.he 1 sibling, 1 reply; 16+ messages in thread From: Ville Syrjälä @ 2015-08-27 12:15 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 0/3] follow-up 2015-08-27 12:15 ` Ville Syrjälä @ 2015-08-27 13:23 ` Jani Nikula 2015-08-27 13:23 ` [PATCH 1/3] drm/i915: ignore link rate in TPS3 selection Jani Nikula ` (2 more replies) 0 siblings, 3 replies; 16+ messages in thread From: Jani Nikula @ 2015-08-27 13:23 UTC (permalink / raw) To: Ville Syrjälä, Jani Nikula; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/3] drm/i915: ignore link rate in TPS3 selection 2015-08-27 13:23 ` [PATCH 0/3] follow-up Jani Nikula @ 2015-08-27 13:23 ` Jani Nikula 2015-08-27 13:23 ` [PATCH 2/3] drm/i915: add yesno utility function Jani Nikula 2015-08-27 13:23 ` [PATCH 3/3] drm/i915: use the yesno helper for logging Jani Nikula 2 siblings, 0 replies; 16+ messages in thread From: Jani Nikula @ 2015-08-27 13:23 UTC (permalink / raw) To: Ville Syrjälä, Jani Nikula; +Cc: intel-gfx 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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/3] drm/i915: add yesno utility function 2015-08-27 13:23 ` [PATCH 0/3] follow-up Jani Nikula 2015-08-27 13:23 ` [PATCH 1/3] drm/i915: ignore link rate in TPS3 selection Jani Nikula @ 2015-08-27 13:23 ` Jani Nikula 2015-08-27 13:28 ` Chris Wilson 2015-08-27 13:23 ` [PATCH 3/3] drm/i915: use the yesno helper for logging Jani Nikula 2 siblings, 1 reply; 16+ messages in thread From: Jani Nikula @ 2015-08-27 13:23 UTC (permalink / raw) To: Ville Syrjälä, Jani Nikula; +Cc: intel-gfx 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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] drm/i915: add yesno utility function 2015-08-27 13:23 ` [PATCH 2/3] drm/i915: add yesno utility function Jani Nikula @ 2015-08-27 13:28 ` Chris Wilson 2015-08-27 13:53 ` Jani Nikula 0 siblings, 1 reply; 16+ messages in thread From: Chris Wilson @ 2015-08-27 13:28 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] drm/i915: add yesno utility function 2015-08-27 13:28 ` Chris Wilson @ 2015-08-27 13:53 ` Jani Nikula 2015-08-31 14:23 ` Jani Nikula 0 siblings, 1 reply; 16+ messages in thread From: Jani Nikula @ 2015-08-27 13:53 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] drm/i915: add yesno utility function 2015-08-27 13:53 ` Jani Nikula @ 2015-08-31 14:23 ` Jani Nikula 2015-08-31 14:33 ` Chris Wilson 0 siblings, 1 reply; 16+ messages in thread From: Jani Nikula @ 2015-08-31 14:23 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] drm/i915: add yesno utility function 2015-08-31 14:23 ` Jani Nikula @ 2015-08-31 14:33 ` Chris Wilson 2015-09-02 9:17 ` Daniel Vetter 0 siblings, 1 reply; 16+ messages in thread From: Chris Wilson @ 2015-08-31 14:33 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] drm/i915: add yesno utility function 2015-08-31 14:33 ` Chris Wilson @ 2015-09-02 9:17 ` Daniel Vetter 0 siblings, 0 replies; 16+ messages in thread From: Daniel Vetter @ 2015-09-02 9:17 UTC (permalink / raw) To: Chris Wilson, Jani Nikula, Ville Syrjälä, intel-gfx 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/3] drm/i915: use the yesno helper for logging 2015-08-27 13:23 ` [PATCH 0/3] follow-up Jani Nikula 2015-08-27 13:23 ` [PATCH 1/3] drm/i915: ignore link rate in TPS3 selection Jani Nikula 2015-08-27 13:23 ` [PATCH 2/3] drm/i915: add yesno utility function Jani Nikula @ 2015-08-27 13:23 ` Jani Nikula 2 siblings, 0 replies; 16+ messages in thread From: Jani Nikula @ 2015-08-27 13:23 UTC (permalink / raw) To: Ville Syrjälä, Jani Nikula; +Cc: intel-gfx 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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 3/3] drm/i915/dp: move TPS3 logic to where it's used 2015-08-27 10:25 ` [PATCH v2 3/3] drm/i915/dp: move TPS3 logic to where it's used Jani Nikula 2015-08-27 12:15 ` Ville Syrjälä @ 2015-08-29 19:00 ` shuang.he 1 sibling, 0 replies; 16+ messages in thread From: shuang.he @ 2015-08-29 19:00 UTC (permalink / raw) To: shuang.he, julianx.dumez, christophe.sureau, lei.a.liu, intel-gfx, jani.nikula 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2015-09-03 8:17 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-08-27 10:25 [PATCH v2 1/3] i915/dp: add intel_dp_tps3_supported helper Jani Nikula 2015-08-27 10:25 ` [PATCH v2 2/3] drm/i915/dp: use the drm dp helper for determining sink tps3 support Jani Nikula 2015-09-01 9:37 ` Daniel Vetter 2015-09-03 8:20 ` Jani Nikula 2015-08-27 10:25 ` [PATCH v2 3/3] drm/i915/dp: move TPS3 logic to where it's used Jani Nikula 2015-08-27 12:15 ` Ville Syrjälä 2015-08-27 13:23 ` [PATCH 0/3] follow-up Jani Nikula 2015-08-27 13:23 ` [PATCH 1/3] drm/i915: ignore link rate in TPS3 selection Jani Nikula 2015-08-27 13:23 ` [PATCH 2/3] drm/i915: add yesno utility function Jani Nikula 2015-08-27 13:28 ` Chris Wilson 2015-08-27 13:53 ` Jani Nikula 2015-08-31 14:23 ` Jani Nikula 2015-08-31 14:33 ` Chris Wilson 2015-09-02 9:17 ` Daniel Vetter 2015-08-27 13:23 ` [PATCH 3/3] drm/i915: use the yesno helper for logging Jani Nikula 2015-08-29 19:00 ` [PATCH v2 3/3] drm/i915/dp: move TPS3 logic to where it's used shuang.he
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.