On Thu, Jan 12, 2017 at 12:37:46PM -0800, Rodrigo Vivi wrote:
> On Tue, Dec 20, 2016 at 03:29:54PM +0200, Ville Syrjälä wrote:
> > On Tue, Dec 20, 2016 at 03:10:53PM +0200, Ander Conselvan De Oliveira wrote:
> > > On Mon, 2016-12-19 at 19:28 +0200, ville.syrj...@linux.intel.com wrote:
> > > > From: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > > >
> > > > Rather than recomptuing the pipe pixel rate on demand everwhere, let's
> > > > just stick the precomputed value into the crtc state.
> > > >
> > > > Signed-off-by: Ville Syrjälä <ville.syrj...@linux.intel.com>
> > > > ---
> > > > drivers/gpu/drm/i915/intel_display.c | 31
> > > > ++++++++++++++++++++++++++-----
> > > > drivers/gpu/drm/i915/intel_drv.h | 2 ++
> > > > drivers/gpu/drm/i915/intel_fbc.c | 3 +--
> > > > drivers/gpu/drm/i915/intel_pm.c | 14 +++++++-------
> > > > 4 files changed, 36 insertions(+), 14 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > > b/drivers/gpu/drm/i915/intel_display.c
> > > > index 0b0d7e8be630..1d979041c52c 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > >
> > >
> > > [...]
> > >
> > > > @@ -16919,7 +16938,7 @@ static void
> > > > intel_modeset_readout_hw_state(struct drm_device *dev)
> > > __drm_atomic_helper_crtc_destroy_state(&crtc_state->base);
> > > memset(crtc_state, 0, sizeof(*crtc_state));
> > > crtc_state->base.crtc = &crtc->base;
> > >
> > > crtc_state->base.active = crtc_state->base.enable =
> > > dev_priv->display.get_pipe_config(crtc,
> > > crtc_state);
> > >
> > > crtc->base.enabled = crtc_state->base.enable;
> > > crtc->active = crtc_state->base.active;
> > >
> > > if (crtc_state->base.active) {
> > > > dev_priv->active_crtcs |= 1 << crtc->pipe;
> > > >
> > > > if (INTEL_GEN(dev_priv) >= 9 ||
> > > > IS_BROADWELL(dev_priv))
> > > > - pixclk =
> > > > ilk_pipe_pixel_rate(crtc_state);
> > > > + pixclk = crtc_state->pixel_rate;
> > >
> > > Aren't you reading 0 here, because of the memset above? As far as I can
> > > tell,
> > > haswell_get_pipe_config() doesn't set crtc_state->pixel_rate.
> >
> > Hmm, yeah. Which means this whole piece of min_pixclk[] code is in
> > the wrong place. You can't know the pixel rate until you know the
> > clock, and you don't know that until you've done the full readout
> > (meaning the encoder .get_config() hooks have been called as well).
>
> Apparently this is the only piece blocking this already reviewed series,
> right?
Not sure. I'll have to go over it again to see what was said. At least
adding some docs for intel_cdclk.c was requested.
>
> So, what are the plans? drop this patch and move with the other patches?
I think I pushed the fix to move the min_pixclk[] handling to happen a
little later, so all I should have to do is rebase this patch and it
should all be good.
But as usual I have a few too many irons in the fire, so I haven't
managed to get back to this series yet.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
