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

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx