On Fri, Oct 17, 2014 at 08:26:13PM +0300, Mika Kuoppala wrote:
> Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> writes:
>
> > On Thu, 16 Oct 2014, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> >> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >>
> >> 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@xxxxxxxxxxxxxxx>
> >> ---
> >> 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?
Imo this is awesome!
>
> adds ~16k to i915.ko.
And for all those concerned about since we need options to cut away _all_
the debug strings. Well _all_ the printk output even. Wasting a bit more
space for easier debug is imo more than worth it.
So please make this a proper patch and let's get this in. For hilarity
maybe cc: lkml and ask get_maintainers who might be interested in a
generic version ... But I think we want the i915 one in asap.
Cheers, Daniel
>
> --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@xxxxxxxxxxxxxxxxxxxxx
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Jani Nikula, Intel Open Source Technology Center
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
