On Thu, 30 Sep 2021, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> On Thu, Sep 30, 2021 at 12:22:58PM +0300, Jani Nikula wrote:
> <snip>
>> diff --git a/drivers/gpu/drm/i915/display/intel_fdi.c b/drivers/gpu/drm/i915/display/intel_fdi.c
>> index af01d1fa761e..02d3294bad7b 100644
>> --- a/drivers/gpu/drm/i915/display/intel_fdi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_fdi.c
>> @@ -10,6 +10,97 @@
>> #include "intel_fdi.h"
>> #include "intel_sideband.h"
>>
>> +static void assert_fdi_tx(struct drm_i915_private *dev_priv,
>> + enum pipe pipe, bool state)
>> +{
>> + bool cur_state;
>> +
>> + if (HAS_DDI(dev_priv)) {
>> + /*
>> + * DDI does not have a specific FDI_TX register.
>> + *
>> + * FDI is never fed from EDP transcoder
>> + * so pipe->transcoder cast is fine here.
>> + */
>> + enum transcoder cpu_transcoder = (enum transcoder)pipe;
>> + cur_state = intel_de_read(dev_priv, TRANS_DDI_FUNC_CTL(cpu_transcoder)) & TRANS_DDI_FUNC_ENABLE;
>> + } else {
>> + cur_state = intel_de_read(dev_priv, FDI_TX_CTL(pipe)) & FDI_TX_ENABLE;
>> + }
>> + I915_STATE_WARN(cur_state != state,
>> + "FDI TX state assertion failure (expected %s, current %s)\n",
>> + onoff(state), onoff(cur_state));
>> +}
>> +
>> +void assert_fdi_tx_enabled(struct drm_i915_private *i915, enum pipe pipe)
>> +{
>> + assert_fdi_tx(i915, pipe, true);
>> +}
>> +
>> +void assert_fdi_tx_disabled(struct drm_i915_private *i915, enum pipe pipe)
>> +{
>> + assert_fdi_tx(i915, pipe, false);
>> +}
>
> For these wrappers I could argue that static inlines would be less
> loc overall, while still wouldn't need any extra struct definitions/etc.
> in the header. But not performance sensitive so from that pov static
> inline is pointless.
I didn't actually check the compiler output, but I think even
performance wise it'll probably end up being just one function call
either way. It's just a question which side of the call the logic
is. But agreed, doesn't really matter.
Anyway, the main argument I have for avoiding static inlines is to not
set an example to cargo cult from. They should be the exception, not the
rule. I think both the driver and the team have grown big enough to
require a style that promotes better structure. Because let's face it,
people look at what's there, copy the style, and not think of all the
subtleties.
> Anyways, this approach seems fine to me. For the series
> Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Thanks,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
