[PATCH] drm/i915: Set mode->crtc_clock during hardware state readout

[PATCH] drm/i915: Set mode->crtc_clock during hardware state readout

[Matt Roper]

intel_mode_from_pipe_config() fills in a mode structure from the CRTC
state that was read out of the hardware, but does not set the
.crtc_clock field (it only sets the .clock).  This causes the subsequent
call to drm_calc_timestamping_constants() to complain with messages like
"*ERROR* crtc 21: Can't calculate constants, dotclock = 0!"  Ensuring
.crtc_clock is set as well eliminates this error.

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index bbeb6d3..4e481e3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7752,6 +7752,7 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode,
 	mode->type = DRM_MODE_TYPE_DRIVER;
 
 	mode->clock = pipe_config->base.adjusted_mode.crtc_clock;
+	mode->crtc_clock = pipe_config->base.adjusted_mode.crtc_clock;
 	mode->flags |= pipe_config->base.adjusted_mode.flags;
 
 	mode->hsync = drm_mode_hsync(mode);
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Set mode->crtc_clock during hardware state readout

[Ville Syrjälä]

On Tue, Oct 06, 2015 at 09:26:31AM -0700, Matt Roper wrote:
> intel_mode_from_pipe_config() fills in a mode structure from the CRTC
> state that was read out of the hardware, but does not set the
> .crtc_clock field (it only sets the .clock).  This causes the subsequent
> call to drm_calc_timestamping_constants() to complain with messages like
> "*ERROR* crtc 21: Can't calculate constants, dotclock = 0!"  Ensuring
> .crtc_clock is set as well eliminates this error.
> 
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index bbeb6d3..4e481e3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7752,6 +7752,7 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode,
>  	mode->type = DRM_MODE_TYPE_DRIVER;
>  
>  	mode->clock = pipe_config->base.adjusted_mode.crtc_clock;
> +	mode->crtc_clock = pipe_config->base.adjusted_mode.crtc_clock;

You should never look at crtc_clock unless you're looking at the
adjusted mode. Are you actually seeing these errors with the current
code? They should have been fixed by:

7f4c62840cc4 drm/i915: Assign hwmode after encoder state readout
0f64614dde17 drm/i915: Fix clock readout when pipes are enabled w/o ports

>  	mode->flags |= pipe_config->base.adjusted_mode.flags;
>  
>  	mode->hsync = drm_mode_hsync(mode);
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Set mode->crtc_clock during hardware state readout

[Daniel Vetter]

On Tue, Oct 06, 2015 at 09:26:31AM -0700, Matt Roper wrote:
> intel_mode_from_pipe_config() fills in a mode structure from the CRTC
> state that was read out of the hardware, but does not set the
> .crtc_clock field (it only sets the .clock).  This causes the subsequent
> call to drm_calc_timestamping_constants() to complain with messages like
> "*ERROR* crtc 21: Can't calculate constants, dotclock = 0!"  Ensuring
> .crtc_clock is set as well eliminates this error.
> 
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>

Is this fixing the bug Paulo reported? Why is he not on CC? Why is there
no citation of the commit which broke this?
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index bbeb6d3..4e481e3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7752,6 +7752,7 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode,
>  	mode->type = DRM_MODE_TYPE_DRIVER;
>  
>  	mode->clock = pipe_config->base.adjusted_mode.crtc_clock;
> +	mode->crtc_clock = pipe_config->base.adjusted_mode.crtc_clock;
>  	mode->flags |= pipe_config->base.adjusted_mode.flags;
>  
>  	mode->hsync = drm_mode_hsync(mode);
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Set mode->crtc_clock during hardware state readout

[Matt Roper]

On Wed, Oct 07, 2015 at 04:26:03PM +0200, Daniel Vetter wrote:
> On Tue, Oct 06, 2015 at 09:26:31AM -0700, Matt Roper wrote:
> > intel_mode_from_pipe_config() fills in a mode structure from the CRTC
> > state that was read out of the hardware, but does not set the
> > .crtc_clock field (it only sets the .clock).  This causes the subsequent
> > call to drm_calc_timestamping_constants() to complain with messages like
> > "*ERROR* crtc 21: Can't calculate constants, dotclock = 0!"  Ensuring
> > .crtc_clock is set as well eliminates this error.
> > 
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> 
> Is this fixing the bug Paulo reported? Why is he not on CC? Why is there
> no citation of the commit which broke this?
> -Daniel

This isn't related to the watermark issue he reported, so no (unless
this is a different issue he brought up elsewhere that I haven't seen).

Ville already pointed out that this might already be fixed properly on
nightly; I think I might have got my trees mixed up while building, so
this might not be necessary at all (I'll need to double check today).
But my original thinking (possibly wrong) was that this was just
something we never initialized from day 1, so there was no specific
commit to cite.

Anyway, I think you can probably disregard this patch for now.


Matt

> 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index bbeb6d3..4e481e3 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -7752,6 +7752,7 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode,
> >  	mode->type = DRM_MODE_TYPE_DRIVER;
> >  
> >  	mode->clock = pipe_config->base.adjusted_mode.crtc_clock;
> > +	mode->crtc_clock = pipe_config->base.adjusted_mode.crtc_clock;
> >  	mode->flags |= pipe_config->base.adjusted_mode.flags;
> >  
> >  	mode->hsync = drm_mode_hsync(mode);
> > -- 
> > 2.1.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx