[Intel-gfx] [PATCH 4/7] drm/i915/icl: Add Voltage swing table for MG PHY DDI Buffer

[Paulo Zanoni]

Em Ter, 2018-04-10 às 22:01 +0100, Chris Wilson escreveu:
> Quoting Paulo Zanoni (2018-04-10 21:39:31)
> > Em Ter, 2018-04-10 às 09:51 +0100, Chris Wilson escreveu:
> > > Quoting Paulo Zanoni (2018-03-23 17:24:16)
> > > > From: Manasi Navare <manasi.d.navare at intel.com>
> > > > 
> > > > This table is used for voltage swing programming sequence
> > > > during
> > > > DDI
> > > > Buffer initialization for MG PHY DDI Buffers on Icelake.
> > > 
> > > Except it is not used at all...
> > 
> > It's going to be used later in the series.
> > 
> > > 
> > > drivers/gpu/drm/i915/intel_ddi.c:601:46: error:
> > > ‘icl_mg_phy_ddi_translations’ defined but not used [-
> > > Werror=unused-
> > > const-variable=]
> > 
> > I have all of the I915 debugging options enabled, including
> > CONFIG_DRM_I915_WERROR=y, and I don't get this error. I'm using
> > Fedora
> > 27's gcc. Would it be the case that you have somehow enabled the
> > unused-const-variable warning through some non-traditional way
> > that's
> > not upstream?
> 
> Try make W=1 (which includes kerneldoc checks!). Or clang (don't try
> clang unless you are a masochist, especially not right now as -Wvla
> upsets it).
>  
> > If that's the case, and if we decide that we want Werror=unused-
> > const-
> > variable to block patches from being merged, then I think we should
> > put
> > this error/warning under the i915 debugging .config options and
> > force
> > CI to also use them and tell us about them.
> 
> I'm trying to get W=1 as part of the pre-merge warning set, at the
> same
> level of severity as ignoring checkpatch.

Good. Perhaps some CONFIG_I915_DEBUG_SOMETHING option could also force
W=1 for us so developers get it for free?

>  
> > Because I'm pretty sure if we start enabling random gcc
> > warning/error
> > flags we'll be able to block a huge number of patches from being
> > upstreamed. I just don't think this is something we should do.
> 
> We do. Our code is clean at W=1 except for the odd mistake.

Ok, I'm convinced. I'll add W=1 to my compilation script. If you want
we can revert this patch and I'll squash it to the patch that actually
uses the table.


>  Let's keep
> it that way. As new gcc warnings are developed, we will squash petty
> nuisances and sometimes outright bugs from the code (it has happened
> before and will happen again).
> -Chris