On Mon, Oct 01, 2018 at 11:35:05PM +0300, Ville Syrjälä wrote:
> On Mon, Sep 24, 2018 at 06:12:27PM +0200, Daniel Vetter wrote:
> > On Thu, Sep 20, 2018 at 09:51:44PM +0300, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > >
> > > Check the infoframes and infoframe enable state when comparing two
> > > crtc states.
> > >
> > > We'll use the infoframe logging functions from video/hdmi.c to
> > > show the infoframes as part of the state dump.
> > >
> > > TODO: Try to better integrate the infoframe dumps with
> > > drm state dumps
> > >
> > > v2: drm_printk() is no more
> > >
> > > Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > > ---
> >
> > Might need adapting to PIPE_CONFIG_QUIRK_INFOFRAME, but aside from that
> >
> > Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch>
> >
> > > drivers/gpu/drm/i915/intel_display.c | 49
> > > +++++++++++++++++++++++++++++++++++-
> > > 1 file changed, 48 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index fbcc56caffb6..3dce49e36a05 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -11380,6 +11380,37 @@ intel_compare_link_m_n(const struct
> > > intel_link_m_n *m_n,
> > > return false;
> > > }
> > >
> > > +static bool
> > > +intel_compare_infoframe(const union hdmi_infoframe *a,
> > > + const union hdmi_infoframe *b)
> > > +{
> > > + return memcmp(a, b, sizeof(*a)) == 0;
> > > +}
> > > +
> > > +static void
> > > +pipe_config_infoframe_err(struct drm_i915_private *dev_priv,
> > > + bool adjust, const char *name,
> > > + const union hdmi_infoframe *a,
> > > + const union hdmi_infoframe *b)
> > > +{
> > > + if (adjust) {
> > > + if ((drm_debug & DRM_UT_KMS) == 0)
> > > + return;
> > > +
> > > + drm_dbg(DRM_UT_KMS, "mismatch in %s infoframe", name);
> > > + drm_dbg(DRM_UT_KMS, "expected:");
> > > + hdmi_infoframe_log(KERN_DEBUG, dev_priv->drm.dev, a);
> > > + drm_dbg(DRM_UT_KMS, "found");
> > > + hdmi_infoframe_log(KERN_DEBUG, dev_priv->drm.dev, b);
> > > + } else {
> > > + drm_err("mismatch in %s infoframe", name);
> > > + drm_err("expected:");
> > > + hdmi_infoframe_log(KERN_ERR, dev_priv->drm.dev, a);
> > > + drm_err("found");
> > > + hdmi_infoframe_log(KERN_ERR, dev_priv->drm.dev, b);
> > > + }
> >
> > Mildly concerned about padding fields (since these are the not-compatified
> > structs). Maybe dump the mismatching byte too, plus byte offset? Or maybe
> > I'm just too paranoid.
>
> Not sure. Maybe just wait and see how crazy the compiler really is?
> Apart from the dmesg noise no real harm should befall the user from
> getting this wrong.
The scenario I have in mind is padding fields + someone didn't use
kzalloc. Dumping the mismatching byte would help in debugging this.
-Daniel
>
> >
> > > +}
> > > +
> > > static void __printf(3, 4)
> > > pipe_config_err(bool adjust, const char *name, const char *format, ...)
> > > {
> > > @@ -11541,7 +11572,17 @@ intel_pipe_config_compare(struct
> > > drm_i915_private *dev_priv,
> > > } \
> > > } while (0)
> > >
> > > -#define PIPE_CONF_QUIRK(quirk) \
> > > +#define PIPE_CONF_CHECK_INFOFRAME(name) do { \
> > > + if (!intel_compare_infoframe(¤t_config->infoframes.name, \
> > > + &pipe_config->infoframes.name)) { \
> > > + pipe_config_infoframe_err(dev_priv, adjust, __stringify(name), \
> > > + ¤t_config->infoframes.name, \
> > > + &pipe_config->infoframes.name); \
> > > + ret = false; \
> > > + } \
> > > +} while (0)
> > > +
> > > +#define PIPE_CONF_QUIRK(quirk) \
> > > ((current_config->quirks | pipe_config->quirks) & (quirk))
> > >
> > > PIPE_CONF_CHECK_I(cpu_transcoder);
> > > @@ -11670,6 +11711,12 @@ intel_pipe_config_compare(struct
> > > drm_i915_private *dev_priv,
> > >
> > > PIPE_CONF_CHECK_I(min_voltage_level);
> > >
> > > + PIPE_CONF_CHECK_X(infoframes.enable);
> > > + PIPE_CONF_CHECK_X(infoframes.gcp);
> > > + PIPE_CONF_CHECK_INFOFRAME(avi);
> > > + PIPE_CONF_CHECK_INFOFRAME(spd);
> > > + PIPE_CONF_CHECK_INFOFRAME(hdmi);
> > > +
> > > #undef PIPE_CONF_CHECK_X
> > > #undef PIPE_CONF_CHECK_I
> > > #undef PIPE_CONF_CHECK_BOOL
> > > --
> > > 2.16.4
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > intel-...@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
>
> --
> Ville Syrjälä
> Intel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
