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

Mika Kuoppala
2014-10-17T17:27:05Z

Jani Nikula <jani.nikula@linux.intel.com> writes:

> 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.
>

Is

diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index ac6232b..9b82d90 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -57,6 +57,11 @@
#define DRIVER_DESC "Intel Graphics"
#define DRIVER_DATE "20141003"

+#ifdef WARN_ON
+#undef WARN_ON
+#define WARN_ON(x) WARN(x, "WARN_ON(" #x ")")
+#endif
+

too big of a hammer?

adds ~16k to i915.ko.

--Mika

> 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