On Thu, Sep 22, 2022 at 11:17:01AM +0300, Jani Nikula wrote:
> On Wed, 21 Sep 2022, Gustavo Sousa <gustavo.sousa@xxxxxxxxx> wrote:
> > On Wed, Sep 21, 2022 at 11:21:00AM +0300, Jani Nikula wrote:
> >> On Tue, 20 Sep 2022, Gustavo Sousa <gustavo.sousa@xxxxxxxxx> wrote:
> >> > Hi, Jani.
> >> >
> >> > On Tue, Sep 20, 2022 at 10:19:53AM +0300, Jani Nikula wrote:
> >> >> On Mon, 19 Sep 2022, Gustavo Sousa <gustavo.sousa@xxxxxxxxx> wrote:
> >> >> > Make the code more readable, which will be more apparent as new
> >> >> > platforms with different hotplug inversion needs are added.
> >> >> >
> >> >> > Signed-off-by: Gustavo Sousa <gustavo.sousa@xxxxxxxxx>
> >> >> > ---
> >> >> > drivers/gpu/drm/i915/i915_irq.c | 25 ++++++++++++++++---------
> >> >> > 1 file changed, 16 insertions(+), 9 deletions(-)
> >> >> >
> >> >> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> >> >> > index de06f293e173..c53d21ae197f 100644
> >> >> > --- a/drivers/gpu/drm/i915/i915_irq.c
> >> >> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> >> >> > @@ -3263,6 +3263,21 @@ static void cherryview_irq_reset(struct drm_i915_private *dev_priv)
> >> >> > spin_unlock_irq(&dev_priv->irq_lock);
> >> >> > }
> >> >> >
> >> >> > +static void setup_hotplug_inversion(struct drm_i915_private *dev_priv)
> >> >> > +{
> >> >> > + u32 invert_bits;
> >> >> > +
> >> >> > + if (HAS_PCH_DG1(dev_priv))
> >> >> > + invert_bits = INVERT_DDIA_HPD |
> >> >> > + INVERT_DDIB_HPD |
> >> >> > + INVERT_DDIC_HPD |
> >> >> > + INVERT_DDID_HPD;
Just checked the source and it seems I'm the only oddball aligning stuff like
this :-)
> >> >>
> >> >> Nitpick, the indentation will be off compared to automated indentation.
> >> >
> >> > What do you mean by automated indentation?
> >>
> >> For example, hit TAB on the lines using a smart enough editor, which has
> >> been configured to follow kernel coding style. ;)
> >>
> >
> > I'm not sure I completely understand the issue. Could you provide an example of
> > such a configuration?
>
> I never indent anything manually. If I hit TAB in emacs (or run M-x
> indent-region), it'll indent the current line (or region) using the
> built-in "linux" style [1].
>
> [1] https://www.gnu.org/software/emacs/manual/html_node/ccmode/Built_002din-Styles.html
>
> > Is the problem here the spaces after the tabs to align each INVERT_DDI*? Would
> > you suggest I just use tabs and do not align them to the first one?
>
> If a parenthesized element in round brackets is split to multiple lines,
> the indent is usually tabs first and then spaces to align everything
> after the open parenthesis. If there are no parenthesis, the indent is
> just tabs.
>
> Guess why the original dg1_hpd_irq_setup() has round brackets in the
> statement. ;) Though not everyone likes superfluous parenthesis for
> style.
>
> > E.g.:
> >
> > invert_bits = INVERT_DDIA_HPD |
> > INVERT_DDIB_HPD |
> > INVERT_DDIC_HPD |
> > INVERT_DDID_HPD;
> >
> > Another one:
> >
> > invert_bits = INVERT_DDIA_HPD |
> > INVERT_DDIB_HPD |
> > INVERT_DDIC_HPD |
> > INVERT_DDID_HPD;
>
> Your patch applied, re-indented, and tabs converted to spaces for visual
> here:
>
> if (HAS_PCH_DG1(dev_priv))
> invert_bits = INVERT_DDIA_HPD |
> INVERT_DDIB_HPD |
> INVERT_DDIC_HPD |
> INVERT_DDID_HPD;
>
> Anyway, indentation isn't a very fruitful conversation in itself. The
> original nitpick was an off the cuff remark. I'm mostly trying to
> suggest to use tools that will do indentation and other things for you,
> so you don't have to worry about getting it right.
>
Thank you for the tips! I find them very helpful. I'll make sure to use proper
style when sending a new version.
--
Gustavo Sousa
