public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@linux.intel.com>
To: ville.syrjala@linux.intel.com, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 01/17] drm/i915: Warn if trying to register eDP	on port != B/C on vlv/chv
Date: Fri, 17 Oct 2014 12:47:31 +0300	[thread overview]
Message-ID: <87y4sfkoy4.fsf@intel.com> (raw)
In-Reply-To: <1413484055-20669-2-git-send-email-ville.syrjala@linux.intel.com>

On Thu, 16 Oct 2014, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Only ports B and C have the power sequencer and backlight controls,
> so complain if we ever try to register an eDP connector on some other
> port.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 4455009..de919e9 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5222,6 +5222,11 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
>  	if (type == DRM_MODE_CONNECTOR_eDP)
>  		intel_encoder->type = INTEL_OUTPUT_EDP;
>  
> +	/* eDP only on port B and/or C on vlv/chv */
> +	if (WARN_ON(IS_VALLEYVIEW(dev) && is_edp(intel_dp) &&
> +		    port != PORT_B && port != PORT_C))
> +		return false;

Sidetracking about WARNs here for a sec.

One takeaway from working on bugs for a month is that when you read a
lot (I mean really a lot) of dmesgs for various kernel versions, and you
see all those warning backtraces from WARN_ON in the logs, it gets
really tedious to find the line emitting the warning in the source. You
see the function and the line number, but maybe those have changed and
maybe the warn was different a few kernel releases ago.

The WARNs with a descriptive message, on the other hand, are really
quite helpful both while reading the backtrace and while reading the
source. Kind of self-documenting. I'd like to discourage the use of
WARN_ON in favor of WARN all around.

So here you have a WARN_ON with a comment that could become the WARN
message almost verbatim. Please consider making the change.

BR,
Jani.



> +
>  	DRM_DEBUG_KMS("Adding %s connector on port %c\n",
>  			type == DRM_MODE_CONNECTOR_eDP ? "eDP" : "DP",
>  			port_name(port));
> -- 
> 2.0.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

  reply	other threads:[~2014-10-17  9:47 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-16 18:27 [PATCH 00/17] drm/i915: Fix vlv/chv panel power sequencer ville.syrjala
2014-10-16 18:27 ` [PATCH 01/17] drm/i915: Warn if trying to register eDP on port != B/C on vlv/chv ville.syrjala
2014-10-17  9:47   ` Jani Nikula [this message]
2014-10-17 11:28     ` Ville Syrjälä
2014-10-17 17:26     ` Mika Kuoppala
2014-10-21 15:42       ` Daniel Vetter
2014-10-16 18:27 ` [PATCH 02/17] drm/i915: Warn if stealing power sequencer from an active eDP port ville.syrjala
2014-10-28  8:10   ` Daniel Vetter
2014-10-28  8:14     ` Ville Syrjälä
2014-10-28  8:34       ` Daniel Vetter
2014-10-28  9:07         ` Ville Syrjälä
2014-10-28 10:30           ` Daniel Vetter
2014-10-16 18:27 ` [PATCH 03/17] drm/i915: Remove high level intel_edp_vdd_{on, off}() from hpd/detect ville.syrjala
2014-10-16 18:27 ` [PATCH 04/17] drm/i915: Store power sequencer delays in intel_dp ville.syrjala
2014-10-16 18:27 ` [PATCH 05/17] drm/i915: Don't initialize power seqeuencer delays more than once ville.syrjala
2014-10-27 14:43   ` Imre Deak
2014-10-27 14:55     ` Ville Syrjälä
2014-10-28  8:12       ` Daniel Vetter
2014-10-16 18:27 ` [PATCH 06/17] drm/i915: Split power sequencer panel on/off functions to locked and unlocked variants ville.syrjala
2014-10-16 18:27 ` [PATCH 07/17] drm/i915: Hold the pps mutex across the whole panel power enable sequence ville.syrjala
2014-10-16 18:27 ` [PATCH 08/17] drm/i915: Wait for PHY port ready before link training on VLV/CHV ville.syrjala
2014-10-22 15:10   ` Todd Previte
2014-10-28  8:15     ` Daniel Vetter
2014-11-04 21:58       ` Todd Previte
2014-10-16 18:27 ` [PATCH 09/17] drm/i915: Fix eDP link training when switching pipes " ville.syrjala
2014-10-16 18:29 ` [PATCH 10/17] drm/i915: Kick the power sequencer before AUX transactions ville.syrjala
2014-10-16 18:29 ` [PATCH 11/17] drm/i915: Make sure DPLL is enabled when kicking the power sequencer on VLV/CHV ville.syrjala
2014-10-28  8:22   ` Daniel Vetter
2014-10-28  8:27     ` Ville Syrjälä
2014-10-28  8:55   ` [PATCH v2 " ville.syrjala
2014-10-28 11:20     ` [PATCH v3 " ville.syrjala
2014-10-16 18:29 ` [PATCH 12/17] drm/i915: Don't kick the power seqeuncer just to check if we have vdd/panel power ville.syrjala
2014-10-27 17:10   ` Imre Deak
2014-10-28  8:03     ` Ville Syrjälä
2014-10-28  8:07       ` Daniel Vetter
2014-10-28  8:26       ` Daniel Vetter
2014-10-16 18:29 ` [PATCH 13/17] drm/i915: Clear PPS port select when giving up the power sequencer ville.syrjala
2014-10-16 18:29 ` [PATCH 14/17] drm/i915: Warn if stealing non pipe A/B " ville.syrjala
2014-10-16 18:29 ` [PATCH 15/17] drm/i915: Steal power sequencer in vlv_power_sequencer_pipe() ville.syrjala
2014-10-28  8:30   ` Daniel Vetter
2014-10-16 18:30 ` [PATCH 16/17] drm/i915: Improve VDD/PPS debugs ville.syrjala
2014-10-16 18:30 ` [PATCH 17/17] drm/i915: Warn if panel power is already on when enabling it ville.syrjala
2014-10-27 17:56 ` [PATCH 00/17] drm/i915: Fix vlv/chv panel power sequencer Imre Deak
2014-10-28  8:32   ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87y4sfkoy4.fsf@intel.com \
    --to=jani.nikula@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=ville.syrjala@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox