From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 01/17] drm/i915: Warn if trying to register eDP on port != B/C on vlv/chv Date: Tue, 21 Oct 2014 17:42:02 +0200 Message-ID: <20141021154202.GV26941@phenom.ffwll.local> References: <1413484055-20669-1-git-send-email-ville.syrjala@linux.intel.com> <1413484055-20669-2-git-send-email-ville.syrjala@linux.intel.com> <87y4sfkoy4.fsf@intel.com> <87oata6216.fsf@gaia.fi.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-wg0-f50.google.com (mail-wg0-f50.google.com [74.125.82.50]) by gabe.freedesktop.org (Postfix) with ESMTP id 297D089471 for ; Tue, 21 Oct 2014 08:41:55 -0700 (PDT) Received: by mail-wg0-f50.google.com with SMTP id a1so1682285wgh.21 for ; Tue, 21 Oct 2014 08:41:54 -0700 (PDT) Content-Disposition: inline In-Reply-To: <87oata6216.fsf@gaia.fi.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Mika Kuoppala Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Fri, Oct 17, 2014 at 08:26:13PM +0300, Mika Kuoppala wrote: > Jani Nikula writes: > = > > On Thu, 16 Oct 2014, ville.syrjala@linux.intel.com wrote: > >> From: Ville Syrj=E4l=E4 > >> > >> 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=E4l=E4 > >> --- > >> 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/in= tel_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_po= rt *intel_dig_port, > >> if (type =3D=3D DRM_MODE_CONNECTOR_eDP) > >> intel_encoder->type =3D 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 !=3D PORT_B && port !=3D 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 =3D=3D 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 -- = Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch