On Thu, 24 Sep 2015, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > On Wed, Sep 23, 2015 at 05:23:27PM +0200, Daniel Vetter wrote: >> On Fri, Sep 18, 2015 at 08:03:56PM +0300, ville.syrjala@xxxxxxxxxxxxxxx wrote: >> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> >> > >> > Make I915_READ and I915_WRITE more type safe by wrapping the register >> > offset in a struct. This should eliminate most of the fumbles we've had >> > with misplaced parens. >> > >> > This only takes care of normal mmio registers. We could extend the idea >> > to other register types and define each with its own struct. That way >> > you wouldn't be able to accidentally pass the wrong thing to a specific >> > register access function. >> > >> > There are a few uglies left: >> > - switch statements don't like structs as case values, even if >> > you case 'case DEADBEEF.reg:'. Fortunately we don't have too many >> > of those so can maybe switch them to use some port enums or such, >> > or if all else fails if ladders >> > - cmd parser is still iffed out. I was just tool lazy to do that >> > for an RFC >> > - the vgpu stuff is ugly cause I didn't spend any time figuring out >> > what it really wants to do >> > - maybe something else I'm overlooking? >> > >> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> >> >> I like this very much, which is also why I started pulling in prep patches already. >> >> One bikeshed though: I think this is one of the very few exceptions where >> coding style says typedefs are ok, since this really is just an opaque >> datatype that gets passed around and only opened-up in low-level code >> similar to ptes. So my vote is on i915_reg_t. And since we might want to >> extend this to other register io functions maybe call it i915_mmio_reg_t. > > Chris wanted the same, and so I've made the change already. If you want to > take sneak peek, it's available in the update branch I mentioned in my earlier > reply to the cover letter. You're too fast, making the change already... ;) IMO the shorter the better for the common case, and I'd argue we can use i915_reg_t for mmio, and something else for the rest. BR, Jani. > > -- > Ville Syrjälä > Intel OTC > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
