| Message ID | 20210908003944.2972024-13-airlied@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | i915/display: split and constify vtable | expand |
On Wed, 08 Sep 2021, Dave Airlie <airlied@gmail.com> wrote: > From: Dave Airlie <airlied@redhat.com> > > Avoid having writeable function pointers. Would benefit from the call wrapper and naming. Reviewed-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 2 +- > drivers/gpu/drm/i915/display/intel_fdi.c | 18 +++++++++++++++--- > drivers/gpu/drm/i915/i915_drv.h | 2 +- > 3 files changed, 17 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 87950202f4ce..0ad577aceb9d 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -2100,7 +2100,7 @@ static void ilk_pch_enable(const struct intel_atomic_state *state, > assert_pch_transcoder_disabled(dev_priv, pipe); > > /* For PCH output, training FDI link */ > - dev_priv->fdi_funcs.fdi_link_train(crtc, crtc_state); > + dev_priv->fdi_funcs->fdi_link_train(crtc, crtc_state); > > /* We need to program the right clock selection before writing the pixel > * mutliplier into the DPLL. */ > diff --git a/drivers/gpu/drm/i915/display/intel_fdi.c b/drivers/gpu/drm/i915/display/intel_fdi.c > index d9f952e0c67f..68aa9c7b18ec 100644 > --- a/drivers/gpu/drm/i915/display/intel_fdi.c > +++ b/drivers/gpu/drm/i915/display/intel_fdi.c > @@ -1005,15 +1005,27 @@ void lpt_fdi_program_mphy(struct drm_i915_private *dev_priv) > intel_sbi_write(dev_priv, 0x21EC, tmp, SBI_MPHY); > } > > +static const struct drm_i915_fdi_link_train_funcs ilk_funcs = { > + .fdi_link_train = ilk_fdi_link_train > +}; > + > +static const struct drm_i915_fdi_link_train_funcs gen6_funcs = { > + .fdi_link_train = gen6_fdi_link_train > +}; > + > +static const struct drm_i915_fdi_link_train_funcs ivb_funcs = { > + .fdi_link_train = ivb_manual_fdi_link_train > +}; > + > void > intel_fdi_init_hook(struct drm_i915_private *dev_priv) > { > if (IS_IRONLAKE(dev_priv)) { > - dev_priv->fdi_funcs.fdi_link_train = ilk_fdi_link_train; > + dev_priv->fdi_funcs = &ilk_funcs; > } else if (IS_SANDYBRIDGE(dev_priv)) { > - dev_priv->fdi_funcs.fdi_link_train = gen6_fdi_link_train; > + dev_priv->fdi_funcs = &gen6_funcs; > } else if (IS_IVYBRIDGE(dev_priv)) { > /* FIXME: detect B0+ stepping and use auto training */ > - dev_priv->fdi_funcs.fdi_link_train = ivb_manual_fdi_link_train; > + dev_priv->fdi_funcs = &ivb_funcs; > } > } > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 461ab0a0f088..b3765222e717 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1006,7 +1006,7 @@ struct drm_i915_private { > struct drm_i915_irq_funcs irq_funcs; > > /* fdi display functions */ > - struct drm_i915_fdi_link_train_funcs fdi_funcs; > + const struct drm_i915_fdi_link_train_funcs *fdi_funcs; > > /* display pll funcs */ > struct drm_i915_dpll_funcs dpll_funcs;
On Wed, 08 Sep 2021, Dave Airlie <airlied@gmail.com> wrote: > From: Dave Airlie <airlied@redhat.com> > > Avoid having writeable function pointers. > --- > drivers/gpu/drm/i915/display/intel_display.c | 2 +- > drivers/gpu/drm/i915/display/intel_fdi.c | 18 +++++++++++++++--- > drivers/gpu/drm/i915/i915_drv.h | 2 +- > 3 files changed, 17 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 87950202f4ce..0ad577aceb9d 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -2100,7 +2100,7 @@ static void ilk_pch_enable(const struct intel_atomic_state *state, > assert_pch_transcoder_disabled(dev_priv, pipe); > > /* For PCH output, training FDI link */ > - dev_priv->fdi_funcs.fdi_link_train(crtc, crtc_state); > + dev_priv->fdi_funcs->fdi_link_train(crtc, crtc_state); > > /* We need to program the right clock selection before writing the pixel > * mutliplier into the DPLL. */ > diff --git a/drivers/gpu/drm/i915/display/intel_fdi.c b/drivers/gpu/drm/i915/display/intel_fdi.c > index d9f952e0c67f..68aa9c7b18ec 100644 > --- a/drivers/gpu/drm/i915/display/intel_fdi.c > +++ b/drivers/gpu/drm/i915/display/intel_fdi.c > @@ -1005,15 +1005,27 @@ void lpt_fdi_program_mphy(struct drm_i915_private *dev_priv) > intel_sbi_write(dev_priv, 0x21EC, tmp, SBI_MPHY); > } > > +static const struct drm_i915_fdi_link_train_funcs ilk_funcs = { > + .fdi_link_train = ilk_fdi_link_train Oh, I guess we could add , at the end of all of these, across all patches, even if some of them currently hold only one member. It's just so much cleaner if ever you need to add another member. BR, Jani. > +}; > + > +static const struct drm_i915_fdi_link_train_funcs gen6_funcs = { > + .fdi_link_train = gen6_fdi_link_train > +}; > + > +static const struct drm_i915_fdi_link_train_funcs ivb_funcs = { > + .fdi_link_train = ivb_manual_fdi_link_train > +}; > + > void > intel_fdi_init_hook(struct drm_i915_private *dev_priv) > { > if (IS_IRONLAKE(dev_priv)) { > - dev_priv->fdi_funcs.fdi_link_train = ilk_fdi_link_train; > + dev_priv->fdi_funcs = &ilk_funcs; > } else if (IS_SANDYBRIDGE(dev_priv)) { > - dev_priv->fdi_funcs.fdi_link_train = gen6_fdi_link_train; > + dev_priv->fdi_funcs = &gen6_funcs; > } else if (IS_IVYBRIDGE(dev_priv)) { > /* FIXME: detect B0+ stepping and use auto training */ > - dev_priv->fdi_funcs.fdi_link_train = ivb_manual_fdi_link_train; > + dev_priv->fdi_funcs = &ivb_funcs; > } > } > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 461ab0a0f088..b3765222e717 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1006,7 +1006,7 @@ struct drm_i915_private { > struct drm_i915_irq_funcs irq_funcs; > > /* fdi display functions */ > - struct drm_i915_fdi_link_train_funcs fdi_funcs; > + const struct drm_i915_fdi_link_train_funcs *fdi_funcs; > > /* display pll funcs */ > struct drm_i915_dpll_funcs dpll_funcs;
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 87950202f4ce..0ad577aceb9d 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -2100,7 +2100,7 @@ static void ilk_pch_enable(const struct intel_atomic_state *state, assert_pch_transcoder_disabled(dev_priv, pipe); /* For PCH output, training FDI link */ - dev_priv->fdi_funcs.fdi_link_train(crtc, crtc_state); + dev_priv->fdi_funcs->fdi_link_train(crtc, crtc_state); /* We need to program the right clock selection before writing the pixel * mutliplier into the DPLL. */ diff --git a/drivers/gpu/drm/i915/display/intel_fdi.c b/drivers/gpu/drm/i915/display/intel_fdi.c index d9f952e0c67f..68aa9c7b18ec 100644 --- a/drivers/gpu/drm/i915/display/intel_fdi.c +++ b/drivers/gpu/drm/i915/display/intel_fdi.c @@ -1005,15 +1005,27 @@ void lpt_fdi_program_mphy(struct drm_i915_private *dev_priv) intel_sbi_write(dev_priv, 0x21EC, tmp, SBI_MPHY); } +static const struct drm_i915_fdi_link_train_funcs ilk_funcs = { + .fdi_link_train = ilk_fdi_link_train +}; + +static const struct drm_i915_fdi_link_train_funcs gen6_funcs = { + .fdi_link_train = gen6_fdi_link_train +}; + +static const struct drm_i915_fdi_link_train_funcs ivb_funcs = { + .fdi_link_train = ivb_manual_fdi_link_train +}; + void intel_fdi_init_hook(struct drm_i915_private *dev_priv) { if (IS_IRONLAKE(dev_priv)) { - dev_priv->fdi_funcs.fdi_link_train = ilk_fdi_link_train; + dev_priv->fdi_funcs = &ilk_funcs; } else if (IS_SANDYBRIDGE(dev_priv)) { - dev_priv->fdi_funcs.fdi_link_train = gen6_fdi_link_train; + dev_priv->fdi_funcs = &gen6_funcs; } else if (IS_IVYBRIDGE(dev_priv)) { /* FIXME: detect B0+ stepping and use auto training */ - dev_priv->fdi_funcs.fdi_link_train = ivb_manual_fdi_link_train; + dev_priv->fdi_funcs = &ivb_funcs; } } diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 461ab0a0f088..b3765222e717 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1006,7 +1006,7 @@ struct drm_i915_private { struct drm_i915_irq_funcs irq_funcs; /* fdi display functions */ - struct drm_i915_fdi_link_train_funcs fdi_funcs; + const struct drm_i915_fdi_link_train_funcs *fdi_funcs; /* display pll funcs */ struct drm_i915_dpll_funcs dpll_funcs;
From: Dave Airlie <airlied@redhat.com> Avoid having writeable function pointers. --- drivers/gpu/drm/i915/display/intel_display.c | 2 +- drivers/gpu/drm/i915/display/intel_fdi.c | 18 +++++++++++++++--- drivers/gpu/drm/i915/i915_drv.h | 2 +- 3 files changed, 17 insertions(+), 5 deletions(-)