Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753320AbdGSIt5 (ORCPT ); Wed, 19 Jul 2017 04:49:57 -0400 Received: from mga01.intel.com ([192.55.52.88]:37079 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751754AbdGSItv (ORCPT ); Wed, 19 Jul 2017 04:49:51 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,380,1496127600"; d="scan'208";a="880393106" From: Jani Nikula To: Paul Kocialkowski , Manasi Navare Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Daniel Vetter , David Airlie Subject: Re: [PATCH v2] drm/i915: Explicit the connector name for DP link training result In-Reply-To: <1500453187.1329.3.camel@linux.intel.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <20170718142536.2306-1-paul.kocialkowski@linux.intel.com> <20170718192036.GB6173@intel.com> <1500453187.1329.3.camel@linux.intel.com> Date: Wed, 19 Jul 2017 11:49:47 +0300 Message-ID: <87pocwhkus.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2970 Lines: 83 On Wed, 19 Jul 2017, Paul Kocialkowski wrote: > On Tue, 2017-07-18 at 12:20 -0700, Manasi Navare wrote: >> On Tue, Jul 18, 2017 at 05:25:36PM +0300, Paul Kocialkowski wrote: >> > This adds the connector name when printing a debug message about the >> > DP >> > link training result. It is useful to figure out what connector is >> > failing when multiple DP connectors are used. >> > >> >> Thanks for the patch, this does make sense during the link training >> failure debugging to know the connector name. >> While at it feel free to change "Failed", "Link Rate, Lane Count", to >> upper case >> in the failure_handling case to be consistent with the pass case. > > Thanks for the review! > > Now that the patch was merged, do you feel like I should make a follow- > up patch to fix consistency in the upper case use or is it enough of a > detail that we can just forget about it? If it bothers you, send a patch, otherwise wait for it to bother someone else enough to send a patch. ;) But please avoid the overuse of all caps for regular words, or capitalization except at the beginning of sentences. I'd go with something like this all over the place: "Link training failed, link rate %d, lane count %d\n" But it hasn't bothered me enough to send a patch. ;) BR, Jani. > >> Reviewed-by: Manasi Navare >> >> Manasi >> >> > Signed-off-by: Paul Kocialkowski >> > --- >> > drivers/gpu/drm/i915/intel_dp_link_training.c | 8 ++++++-- >> > 1 file changed, 6 insertions(+), 2 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c >> > b/drivers/gpu/drm/i915/intel_dp_link_training.c >> > index b79c1c0e404c..05907fa8a553 100644 >> > --- a/drivers/gpu/drm/i915/intel_dp_link_training.c >> > +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c >> > @@ -321,12 +321,16 @@ intel_dp_start_link_train(struct intel_dp >> > *intel_dp) >> > if (!intel_dp_link_training_channel_equalization(intel_dp)) >> > goto failure_handling; >> > >> > - DRM_DEBUG_KMS("Link Training Passed at Link Rate = %d, Lane >> > count = %d", >> > + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Link Training Passed at >> > Link Rate = %d, Lane count = %d", >> > + intel_connector->base.base.id, >> > + intel_connector->base.name, >> > intel_dp->link_rate, intel_dp->lane_count); >> > return; >> > >> > failure_handling: >> > - DRM_DEBUG_KMS("Link Training failed at link rate = %d, lane >> > count = %d", >> > + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Link Training failed at >> > link rate = %d, lane count = %d", >> > + intel_connector->base.base.id, >> > + intel_connector->base.name, >> > intel_dp->link_rate, intel_dp->lane_count); >> > if (!intel_dp_get_link_train_fallback_values(intel_dp, >> > intel_dp- >> > >link_rate, >> > -- >> > 2.13.2 >> > -- Jani Nikula, Intel Open Source Technology Center