Re: [PATCH 01/17] drm/i915: Warn if trying to register eDP on port != B/C on vlv/chv

Jani Nikula
2014-10-17T09:47:57Z

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