| Message ID | cover.1579871655.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm/i915/display: mass conversion to intel_de_*() register accessors | expand |
Quoting Jani Nikula (2020-01-24 13:25:21) > Hey all, > > So I sent [1] to convert some forcewake register accessors... but what if we > just ripped off the bandage once and for all? It's going to hurt, a lot, but > we'd get it done. > > This completely rids us of the "dev_priv" dependency in display/. > > All the patches here are per-file and independent of each other. We could also > pick and apply the ones that are least likely to conflict. > > Opinions? > > > BR, > Jani. > > > PS. I didn't bother looking at the checkpatch warnings this may generate at this > point. I just used the --linux-spacing option for spatch, and closed my eyes. I > completely scripted the generation of the series, apart from just a couple of > build fixes. Yup. Suck it all in, clean up with the usual code refreshes. Schadenfreude-by: Chris Wilson <chris@chris-wilson.co.uk> I've looked at a couple of patches to confirm that it does appear purely mechanical, Acked-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
On Fri, Jan 24, 2020 at 03:25:21PM +0200, Jani Nikula wrote: > Hey all, > > So I sent [1] to convert some forcewake register accessors... but what if we > just ripped off the bandage once and for all? It's going to hurt, a lot, but > we'd get it done. > > This completely rids us of the "dev_priv" dependency in display/. > > All the patches here are per-file and independent of each other. We could also > pick and apply the ones that are least likely to conflict. > > Opinions? > > > BR, > Jani. > > > PS. I didn't bother looking at the checkpatch warnings this may generate at this > point. I just used the --linux-spacing option for spatch, and closed my eyes. I > completely scripted the generation of the series, apart from just a couple of > build fixes. > > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Lucas De Marchi <lucas.demarchi@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > [1] https://patchwork.freedesktop.org/series/72476/ > > > Jani Nikula (33): > drm/i915/icl_dsi: use intel_de_*() functions for register access > drm/i915/audio: use intel_de_*() functions for register access > drm/i915/cdclk: use intel_de_*() functions for register access > drm/i915/color: use intel_de_*() functions for register access > drm/i915/combo_phy: use intel_de_*() functions for register access > drm/i915/crt: use intel_de_*() functions for register access > drm/i915/ddi: use intel_de_*() functions for register access > drm/i915/display: use intel_de_*() functions for register access > drm/i915/display_power: use intel_de_*() functions for register access > drm/i915/dp: use intel_de_*() functions for register access > drm/i915/dpio_phy: use intel_de_*() functions for register access > drm/i915/dpll_mgr: use intel_de_*() functions for register access > drm/i915/dp_mst: use intel_de_*() functions for register access > drm/i915/dsb: use intel_de_*() functions for register access > drm/i915/dvo: use intel_de_*() functions for register access > drm/i915/fbc: use intel_de_*() functions for register access > drm/i915/fifo_underrun: use intel_de_*() functions for register access > drm/i915/gmbus: use intel_de_*() functions for register access > drm/i915/hdcp: use intel_de_*() functions for register access > drm/i915/hdmi: use intel_de_*() functions for register access > drm/i915/lpe_audio: use intel_de_*() functions for register access > drm/i915/lvds: use intel_de_*() functions for register access > drm/i915/overlay: use intel_de_*() functions for register access > drm/i915/panel: use intel_de_*() functions for register access > drm/i915/pipe_crc: use intel_de_*() functions for register access > drm/i915/psr: use intel_de_*() functions for register access > drm/i915/sdvo: use intel_de_*() functions for register access > drm/i915/sprite: use intel_de_*() functions for register access > drm/i915/tv: use intel_de_*() functions for register access > drm/i915/vdsc: use intel_de_*() functions for register access > drm/i915/vga: use intel_de_*() functions for register access > drm/i915/vlv_dsi: use intel_de_*() functions for register access > drm/i915/vlv_dsi_pll: use intel_de_*() functions for register access > > drivers/gpu/drm/i915/display/icl_dsi.c | 271 ++-- > drivers/gpu/drm/i915/display/intel_audio.c | 112 +- > drivers/gpu/drm/i915/display/intel_cdclk.c | 133 +- > drivers/gpu/drm/i915/display/intel_color.c | 204 +-- > .../gpu/drm/i915/display/intel_combo_phy.c | 66 +- > drivers/gpu/drm/i915/display/intel_crt.c | 51 +- > drivers/gpu/drm/i915/display/intel_ddi.c | 474 +++---- > drivers/gpu/drm/i915/display/intel_display.c | 1171 +++++++++-------- > .../drm/i915/display/intel_display_power.c | 294 +++-- > drivers/gpu/drm/i915/display/intel_dp.c | 234 ++-- > drivers/gpu/drm/i915/display/intel_dp_mst.c | 11 +- > drivers/gpu/drm/i915/display/intel_dpio_phy.c | 77 +- > drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 388 +++--- > drivers/gpu/drm/i915/display/intel_dsb.c | 24 +- > drivers/gpu/drm/i915/display/intel_dvo.c | 34 +- > drivers/gpu/drm/i915/display/intel_fbc.c | 106 +- > .../drm/i915/display/intel_fifo_underrun.c | 37 +- > drivers/gpu/drm/i915/display/intel_gmbus.c | 74 +- > drivers/gpu/drm/i915/display/intel_hdcp.c | 142 +- > drivers/gpu/drm/i915/display/intel_hdmi.c | 258 ++-- > .../gpu/drm/i915/display/intel_lpe_audio.c | 14 +- > drivers/gpu/drm/i915/display/intel_lvds.c | 57 +- > drivers/gpu/drm/i915/display/intel_overlay.c | 45 +- > drivers/gpu/drm/i915/display/intel_panel.c | 255 ++-- > drivers/gpu/drm/i915/display/intel_pipe_crc.c | 20 +- > drivers/gpu/drm/i915/display/intel_psr.c | 70 +- > drivers/gpu/drm/i915/display/intel_sdvo.c | 30 +- > drivers/gpu/drm/i915/display/intel_sprite.c | 320 +++-- > drivers/gpu/drm/i915/display/intel_tv.c | 138 +- > drivers/gpu/drm/i915/display/intel_vdsc.c | 413 +++--- > drivers/gpu/drm/i915/display/intel_vga.c | 7 +- > drivers/gpu/drm/i915/display/vlv_dsi.c | 345 ++--- > drivers/gpu/drm/i915/display/vlv_dsi_pll.c | 49 +- > 33 files changed, 3158 insertions(+), 2766 deletions(-) There's a lot of display (watermark) code in intel_pm.c as well, even though it doesn't live in the display/ directory. We should probably pull the watermark stuff out into a separate display/intel_wm.c or something soon, but in the meantime we'll probably want to switch a bunch of that code over to using these new functions. But I guess you can't do that with coccinelle though since there are parts of the file that aren't display-related and shouldn't use the same display helpers. Matt > > -- > 2.20.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Jan 24, 2020 at 01:54:58PM +0000, Chris Wilson wrote: > Quoting Jani Nikula (2020-01-24 13:25:21) > > Hey all, > > > > So I sent [1] to convert some forcewake register accessors... but what if we > > just ripped off the bandage once and for all? It's going to hurt, a lot, but > > we'd get it done. > > > > This completely rids us of the "dev_priv" dependency in display/. > > > > All the patches here are per-file and independent of each other. We could also > > pick and apply the ones that are least likely to conflict. > > > > Opinions? > > > > > > BR, > > Jani. > > > > > > PS. I didn't bother looking at the checkpatch warnings this may generate at this > > point. I just used the --linux-spacing option for spatch, and closed my eyes. I > > completely scripted the generation of the series, apart from just a couple of > > build fixes. > > Yup. Suck it all in, clean up with the usual code refreshes. > Schadenfreude-by: Chris Wilson <chris@chris-wilson.co.uk> > > I've looked at a couple of patches to confirm that it does appear purely > mechanical, > Acked-by: Chris Wilson <chris@chris-wilson.co.uk> Since it is purely mechanical with coccinelle, why not to make in only one patch? Anyway: Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > -Chris > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, 24 Jan 2020, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote: > On Fri, Jan 24, 2020 at 01:54:58PM +0000, Chris Wilson wrote: >> Quoting Jani Nikula (2020-01-24 13:25:21) >> > Hey all, >> > >> > So I sent [1] to convert some forcewake register accessors... but what if we >> > just ripped off the bandage once and for all? It's going to hurt, a lot, but >> > we'd get it done. >> > >> > This completely rids us of the "dev_priv" dependency in display/. >> > >> > All the patches here are per-file and independent of each other. We could also >> > pick and apply the ones that are least likely to conflict. >> > >> > Opinions? >> > >> > >> > BR, >> > Jani. >> > >> > >> > PS. I didn't bother looking at the checkpatch warnings this may generate at this >> > point. I just used the --linux-spacing option for spatch, and closed my eyes. I >> > completely scripted the generation of the series, apart from just a couple of >> > build fixes. >> >> Yup. Suck it all in, clean up with the usual code refreshes. >> Schadenfreude-by: Chris Wilson <chris@chris-wilson.co.uk> >> >> I've looked at a couple of patches to confirm that it does appear purely >> mechanical, >> Acked-by: Chris Wilson <chris@chris-wilson.co.uk> > > Since it is purely mechanical with coccinelle, why not to make in only one patch? Because such a mega patch would conflict before being able to merge. You'd have to block everything else. I don't think I'd be able to merge these in one go either even if we wanted to. > Anyway: > Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Thanks, Jani. > >> -Chris >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoting Jani Nikula (2020-01-24 15:25:21) > Hey all, > > So I sent [1] to convert some forcewake register accessors... but what if we > just ripped off the bandage once and for all? It's going to hurt, a lot, but > we'd get it done. > > This completely rids us of the "dev_priv" dependency in display/. > > All the patches here are per-file and independent of each other. We could also > pick and apply the ones that are least likely to conflict. > > Opinions? Acked-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Regards, Joonas
On Sat, 25 Jan 2020, Jani Nikula <jani.nikula@intel.com> wrote: > On Fri, 24 Jan 2020, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote: >> On Fri, Jan 24, 2020 at 01:54:58PM +0000, Chris Wilson wrote: >>> Quoting Jani Nikula (2020-01-24 13:25:21) >>> > Hey all, >>> > >>> > So I sent [1] to convert some forcewake register accessors... but what if we >>> > just ripped off the bandage once and for all? It's going to hurt, a lot, but >>> > we'd get it done. >>> > >>> > This completely rids us of the "dev_priv" dependency in display/. >>> > >>> > All the patches here are per-file and independent of each other. We could also >>> > pick and apply the ones that are least likely to conflict. >>> > >>> > Opinions? >>> > >>> > >>> > BR, >>> > Jani. >>> > >>> > >>> > PS. I didn't bother looking at the checkpatch warnings this may generate at this >>> > point. I just used the --linux-spacing option for spatch, and closed my eyes. I >>> > completely scripted the generation of the series, apart from just a couple of >>> > build fixes. >>> >>> Yup. Suck it all in, clean up with the usual code refreshes. >>> Schadenfreude-by: Chris Wilson <chris@chris-wilson.co.uk> >>> >>> I've looked at a couple of patches to confirm that it does appear purely >>> mechanical, >>> Acked-by: Chris Wilson <chris@chris-wilson.co.uk> >> >> Since it is purely mechanical with coccinelle, why not to make in only one patch? > > Because such a mega patch would conflict before being able to > merge. You'd have to block everything else. I don't think I'd be able to > merge these in one go either even if we wanted to. Indeed, I started pushing the patches, pushed all that applied, and eight will need a rebase. Thanks for the acks. BR, Jani. > >> Anyway: >> Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Thanks, > Jani. > >> >>> -Chris >>> _______________________________________________ >>> Intel-gfx mailing list >>> Intel-gfx@lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, 24 Jan 2020, Matt Roper <matthew.d.roper@intel.com> wrote: > There's a lot of display (watermark) code in intel_pm.c as well, even > though it doesn't live in the display/ directory. We should probably > pull the watermark stuff out into a separate display/intel_wm.c or > something soon, but in the meantime we'll probably want to switch a > bunch of that code over to using these new functions. But I guess you > can't do that with coccinelle though since there are parts of the file > that aren't display-related and shouldn't use the same display helpers. Yeah, display/ was a clear-cut line. I may have already pushed some patches using intel_de_*_fw from the top level code, but I think it would be better to move all large chunks of code that do display uncore stuff under display/. BR, Jani.