Re: [PATCH 09/12] drm/i915: Use REG_BIT() & co. for BDW+ EU/slice fuse bits

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jani,

On Tue, Feb 25, 2025 at 09:52:41AM +0200, Jani Nikula wrote:
> On Mon, 24 Feb 2025, Andi Shyti <andi.shyti@xxxxxxxxxxxxxxx> wrote:
> > Hi Ville,
> >
> > On Wed, Feb 12, 2025 at 01:19:37AM +0200, Ville Syrjala wrote:
> >> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >> 
> >> Convert the BDW+ EU/slice fuse bits to the modern REG_BIT()/etc.
> >> style.
> >
> > using REG_BIT() and co. doesn't alway make it more readable. In
> > some of the cases below I would have preferred not to use it.
> 
> Interesting. I read through the patch and I thought all of it was
> good. Care to elaborate?

yes you're right, I should have given an example, but I had
already edited the e-mail and I was lazy to get it back.

In any case, this is an example:

-       s_en = intel_uncore_read(uncore, GEN11_GT_SLICE_ENABLE) &
-               GEN11_GT_S_ENA_MASK;
+       s_en = REG_FIELD_GET(GEN11_GT_S_ENA_MASK,
+                            intel_uncore_read(uncore, GEN11_GT_SLICE_ENABLE));


The removed line to me is clearer than the added line.

I'm not saying that it's not good (otherwise I wouldn't have
r-b'ed it), I'm just saying that not always using the REG_*
macros makes the code clearer.

For consistency with the rest of the patch is anyway fine.

Thanks,
Andi



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux