On Wed, 14 Oct 2015, Dave Gordon <[email protected]> wrote:
> On 17/09/15 14:20, Damien Lespiau wrote:
>> Not the first time! not the last time?
>>
>> There is a possibility to use gcc 5's -Wbool-compare to try and compare
>> (reg) in those macros to a constant and gcc will warn that the
>> comparison between a boolean expression and a constant is always either
>> true or false. Maybe.
>
> Since boolean true (1) cannot be a valid argument to this macro, it
> could contain a compile-time check that the parameter is not 1; if
> boolean false (0) happens not to be a valid register address (BSpec says
> MMIO 0 is reserved) the macro could check that the argument is neither
> of these values, and the compiler might then detect that all possible
> paths lead to a compile-time error. Something like this?
The past errors have also been of the form
if (I915_READ(SOME_REG(pipe) & MASK) == val)
which isn't caught by your check. See [1].
BR,
Jani.
[1]
http://mid.gmane.org/[email protected]
>
> #define I915_READ(reg) \
> ({ \
> if (__builtin_constant_p(reg)) { \
> BUILD_BUG_ON((reg) == false); \
> BUILD_BUG_ON((reg) == true); \
> } \
> dev_priv->uncore.funcs.mmio_readl(dev_priv, (reg), true); \
> })
>
> Interestingly, that reported three errors, all in intel_dsi.c where the
> port-selection macros use 0s to fill in dummy elements when less than 3
> ports are being used.
>
> In function ‘intel_dsi_get_hw_state’
> In function ‘intel_dsi_port_disable’
> In function ‘intel_dsi_port_enable’
>
> Other than that, there weren't any cases where a bool constant was being
> passed to this specific macro (as of today).
>
> .Dave.
>
>> Cc: Imre Deak <[email protected]>
>> Signed-off-by: Damien Lespiau <[email protected]>
>> ---
>> drivers/gpu/drm/i915/intel_ddi.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
>> b/drivers/gpu/drm/i915/intel_ddi.c
>> index 4823184..5b600bf 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -2882,7 +2882,7 @@ static bool bxt_ddi_pll_get_hw_state(struct
>> drm_i915_private *dev_priv,
>> * here just read out lanes 0/1 and output a note if lanes 2/3 differ.
>> */
>> hw_state->pcsdw12 = I915_READ(BXT_PORT_PCS_DW12_LN01(port));
>> - if (I915_READ(BXT_PORT_PCS_DW12_LN23(port) != hw_state->pcsdw12))
>> + if (I915_READ(BXT_PORT_PCS_DW12_LN23(port)) != hw_state->pcsdw12)
>> DRM_DEBUG_DRIVER("lane stagger config different for lane 01
>> (%08x) and 23 (%08x)\n",
>> hw_state->pcsdw12,
>> I915_READ(BXT_PORT_PCS_DW12_LN23(port)));
>
>
> _______________________________________________
> Intel-gfx mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
