On Mon, May 18, 2015 at 05:49:23PM +0200, Daniel Vetter wrote:
> On Wed, May 13, 2015 at 10:23:46PM +0200, Maarten Lankhorst wrote:
> > intel_crtc->config will be removed eventually, so use crtc->hwmode.
> > drm_atomic_helper_update_legacy_modeset_state updates hwmode,
> > but crtc->active will eventually be gone too. Set dotclock to zero
> > to indicate the crtc is inactive.
> >
> > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
>
> I think adding a code comment to our assignment of crtc->hw_mode that we
> need this for i915_get_vblank_timestamp (and only for that) would be
> really good. Especially since I can't find it with a quick grep, at least
> in current upstream ;-)
I don't particularly like resurrecting this zombie. Why we can't just use
crtc->state->adjusted_mode (or wherever the current adjusted mode is kept)?
> -Daniel
>
> > ---
> > drivers/gpu/drm/i915/i915_irq.c | 13 ++++++-------
> > drivers/gpu/drm/i915/intel_display.c | 3 +++
> > 2 files changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index ca457317a8ac..9359ea2399f1 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -564,8 +564,7 @@ static u32 i915_get_vblank_counter(struct drm_device *dev, int pipe)
> > u32 high1, high2, low, pixel, vbl_start, hsync_start, htotal;
> > struct intel_crtc *intel_crtc =
> > to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> > - const struct drm_display_mode *mode =
> > - &intel_crtc->config->base.adjusted_mode;
> > + const struct drm_display_mode *mode = &intel_crtc->base.hwmode;
> >
> > htotal = mode->crtc_htotal;
> > hsync_start = mode->crtc_hsync_start;
> > @@ -620,7 +619,7 @@ static int __intel_get_crtc_scanline(struct intel_crtc *crtc)
> > {
> > struct drm_device *dev = crtc->base.dev;
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > - const struct drm_display_mode *mode = &crtc->config->base.adjusted_mode;
> > + const struct drm_display_mode *mode = &crtc->base.hwmode;
> > enum pipe pipe = crtc->pipe;
> > int position, vtotal;
> >
> > @@ -647,14 +646,14 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe,
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> > struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > - const struct drm_display_mode *mode = &intel_crtc->config->base.adjusted_mode;
> > + const struct drm_display_mode *mode = &intel_crtc->base.hwmode;
> > int position;
> > int vbl_start, vbl_end, hsync_start, htotal, vtotal;
> > bool in_vbl = true;
> > int ret = 0;
> > unsigned long irqflags;
> >
> > - if (!intel_crtc->active) {
> > + if (WARN_ON(!mode->crtc_clock)) {
> > DRM_DEBUG_DRIVER("trying to get scanoutpos for disabled "
> > "pipe %c\n", pipe_name(pipe));
> > return 0;
> > @@ -796,7 +795,7 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe,
> > return -EINVAL;
> > }
> >
> > - if (!crtc->state->active) {
> > + if (!crtc->hwmode.crtc_clock) {
> > DRM_DEBUG_KMS("crtc %d is disabled\n", pipe);
> > return -EBUSY;
> > }
> > @@ -805,7 +804,7 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, int pipe,
> > return drm_calc_vbltimestamp_from_scanoutpos(dev, pipe, max_error,
> > vblank_time, flags,
> > crtc,
> > - &to_intel_crtc(crtc)->config->base.adjusted_mode);
> > + &crtc->hwmode);
> > }
> >
> > static bool intel_hpd_irq_event(struct drm_device *dev,
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 9316b0be4f5b..5063ac3d028d 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -11467,6 +11467,9 @@ intel_modeset_update_state(struct drm_atomic_state *state)
> > WARN_ON(crtc->state->enable != intel_crtc_in_use(crtc));
> >
> > to_intel_crtc(crtc)->config = to_intel_crtc_state(crtc->state);
> > +
> > + if (!crtc->state->active)
> > + crtc->hwmode.crtc_clock = 0;
> > }
> >
> > list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
> > --
> > 2.1.0
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
