> > > > +static void write_iet(struct intel_display *display, enum pipe pipe, > > > > + u32 *data) > > > > +{ > > > > + int i; > > > > + > > > > + for (i = 0; i < HISTOGRAM_IET_LENGTH; i++) { > > > > + if (DISPLAY_VER(display) >= 20) > > > > + intel_de_rmw(display, DPST_IE_BIN(pipe), > > > > + DPST_IE_BIN_DATA_MASK, > > > > + DPST_IE_BIN_DATA(data[i])); > > > > + else > > > > + intel_de_rmw(display, DPST_BIN(pipe), > > > > + DPST_BIN_DATA_MASK, > > > > + DPST_BIN_DATA(data[i])); > > > > + > > > > + drm_dbg_atomic(display->drm, "iet_lut[%d]=%x\n", > > > > + i, data[i]); > > > > + } > > > > > > This looks more clean according to me if (DISPLAY_VER(display) >= > > > 20) { > > > register_base = DPST_IE_BIN(pipe); > > > data_mask = DPST_IE_BIN_DATA_MASK; > > > data_temp = DPST_IE_BIN_DATA(data[i]); } else { > > > register_base = DPST_BIN(pipe); > > > data_mask = DPST_BIN_DATA_MASK; > > > data_temp = DPST_BIN_DATA(data[i]); } intel_de_rmw(display, > > > register_base, data_mask, data_temp); > > > drm_dbg_atomic(display->drm, "iet_lut[%d]=%x\n", i, data[i]); > > > > > > > With the above code snippet data_temp will have to be in the for loop > > so as to get the bit mapped value of data[i] > > > > Yes the whole code snippet will be inside the loop itself > In that case I don't see any advantage of this over the present code. If you still insist will do the necessary changes. Thanks and Regards, Arun R Murthy --------------------