On Wed, Sep 25, 2024 at 01:20:56PM +0300, Jani Nikula wrote:
> On Tue, 24 Sep 2024, Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> wrote:
> > Restrict this function to only save and restore registers
> > functionality. Then, that can be moved out later to under
> > display with a proper name.
> >
> > Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/i915_driver.c | 6 ++++++
> > drivers/gpu/drm/i915/i915_suspend.c | 6 ------
> > 2 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> > index 6dc0104a3e36..c5ffcf229f42 100644
> > --- a/drivers/gpu/drm/i915/i915_driver.c
> > +++ b/drivers/gpu/drm/i915/i915_driver.c
> > @@ -60,6 +60,7 @@
> > #include "display/intel_pch_refclk.h"
> > #include "display/intel_pps.h"
> > #include "display/intel_sprite_uapi.h"
> > +#include "display/intel_vga.h"
> > #include "display/skl_watermark.h"
> >
> > #include "gem/i915_gem_context.h"
> > @@ -1167,6 +1168,11 @@ static int i915_drm_resume(struct drm_device *dev)
> > intel_dmc_resume(display);
> >
> > i915_restore_display(dev_priv);
> > +
> > + intel_vga_redisable(display);
> > +
> > + intel_gmbus_reset(dev_priv);
> > +
>
> Hate to be nitpicky, but these are now called for !HAS_DISPLAY()
> too. Maybe the later patches fix it, but here it's not right.
no need for hate, this is what the reviews are for ;)
I totally agree and will fix on the next revision...
>
> I agree with the approach though.
>
> > intel_pps_unlock_regs_wa(display);
> >
> > intel_init_pch_refclk(dev_priv);
> > diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
> > index 9d3d9b983032..fb67b05cd864 100644
> > --- a/drivers/gpu/drm/i915/i915_suspend.c
> > +++ b/drivers/gpu/drm/i915/i915_suspend.c
> > @@ -26,7 +26,6 @@
> >
> > #include "display/intel_de.h"
> > #include "display/intel_gmbus.h"
> > -#include "display/intel_vga.h"
> >
> > #include "i915_drv.h"
> > #include "i915_reg.h"
> > @@ -118,7 +117,6 @@ void i915_save_display(struct drm_i915_private *dev_priv)
> >
> > void i915_restore_display(struct drm_i915_private *dev_priv)
> > {
> > - struct intel_display *display = &dev_priv->display;
> > struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev);
> >
> > if (!HAS_DISPLAY(dev_priv))
> > @@ -134,8 +132,4 @@ void i915_restore_display(struct drm_i915_private *dev_priv)
> > if (GRAPHICS_VER(dev_priv) <= 4)
> > intel_de_write(dev_priv, DSPARB(dev_priv),
> > dev_priv->regfile.saveDSPARB);
> > -
> > - intel_vga_redisable(display);
> > -
> > - intel_gmbus_reset(dev_priv);
> > }
>
> --
> Jani Nikula, Intel
