[Intel-gfx] [PATCH v2 2/4] drm/i915/glk: Fix maximum scaling factor for Geminilake scalers

[Ander Conselvan De Oliveira]

On Thu, 2017-02-23 at 12:04 +0200, Ville Syrjälä wrote:
> On Thu, Feb 23, 2017 at 09:15:58AM +0200, Ander Conselvan de Oliveira wrote:
> > Geminilake can output two pixels per clock, and that affects the maximum
> > scaling factor for its scalers. Take that into account and avoid the
> > following warning:
> > 
> > WARNING: CPU: 1 PID: 593 at drivers/gpu/drm/i915/intel_display.c:13223 skl_max_scale.part.129+0x78/0x80 [i915]
> > WARN_ON_ONCE(!crtc_clock || cdclk < crtc_clock)
> > Modules linked in: x86_pkg_temp_thermal i915 coretemp kvm_intel kvm i2c_algo_bit drm_kms_helper irqbypass crct10dif_pclmul prime_numbers crc32_pclmul drm ghash_clmulni_intel shpchp tpm_tis tpm_tis_core tpm nfsd authw
> > CPU: 1 PID: 593 Comm: kworker/u8:3 Tainted: G        W       4.10.0-rc8ander+ #330
> > Hardware name: Intel Corp. Geminilake/GLK RVP1 DDR4 (05), BIOS GELKRVPA.X64.0035.B33.1702150552 02/15/2017
> > Workqueue: events_unbound async_run_entry_fn
> > Call Trace:
> >  dump_stack+0x86/0xc3
> >  __warn+0xcb/0xf0
> >  warn_slowpath_fmt+0x5f/0x80
> >  skl_max_scale.part.129+0x78/0x80 [i915]
> >  intel_check_primary_plane+0xa6/0xc0 [i915]
> >  intel_plane_atomic_check_with_state+0xd1/0x1a0 [i915]
> >  ? drm_printk+0xb5/0xc0 [drm]
> >  intel_plane_atomic_check+0x3d/0x80 [i915]
> >  drm_atomic_helper_check_planes+0x7c/0x200 [drm_kms_helper]
> >  intel_atomic_check+0xa5b/0x11a0 [i915]
> >  drm_atomic_check_only+0x353/0x600 [drm]
> >  ? drm_atomic_add_affected_connectors+0x10c/0x120 [drm]
> >  drm_atomic_commit+0x18/0x50 [drm]
> >  restore_fbdev_mode+0x14c/0x2a0 [drm_kms_helper]
> >  drm_fb_helper_restore_fbdev_mode_unlocked+0x34/0x80 [drm_kms_helper]
> >  drm_fb_helper_set_par+0x2d/0x60 [drm_kms_helper]
> >  intel_fbdev_set_par+0x1a/0x70 [i915]
> >  fbcon_init+0x582/0x610
> >  visual_init+0xd6/0x130
> >  do_bind_con_driver+0x1da/0x3c0
> >  do_take_over_console+0x116/0x180
> >  do_fbcon_takeover+0x5c/0xb0
> >  fbcon_event_notify+0x772/0x8a0
> >  ? __blocking_notifier_call_chain+0x35/0x70
> >  notifier_call_chain+0x4a/0x70
> >  __blocking_notifier_call_chain+0x4d/0x70
> >  blocking_notifier_call_chain+0x16/0x20
> >  fb_notifier_call_chain+0x1b/0x20
> >  register_framebuffer+0x278/0x360
> >  drm_fb_helper_initial_config+0x253/0x440 [drm_kms_helper]
> >  intel_fbdev_initial_config+0x18/0x30 [i915]
> >  async_run_entry_fn+0x39/0x170
> >  process_one_work+0x212/0x670
> >  ? process_one_work+0x197/0x670
> >  worker_thread+0x4e/0x490
> >  kthread+0x101/0x140
> >  ? process_one_work+0x670/0x670
> >  ? kthread_create_on_node+0x60/0x60
> >  ret_from_fork+0x31/0x40
> > 
> > v2: s/max_pixclk/max_dotclk/ (Ville)
> > Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira at intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 15 +++++++++++----
> >  1 file changed, 11 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 957c62d..6b65adf 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -13211,16 +13211,22 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
> >  int
> >  skl_max_scale(struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state)
> >  {
> > +	struct drm_i915_private *dev_priv;
> >  	int max_scale;
> > -	int crtc_clock, cdclk;
> > +	int crtc_clock, max_dotclk;
> >  
> >  	if (!intel_crtc || !crtc_state->base.enable)
> >  		return DRM_PLANE_HELPER_NO_SCALING;
> >  
> > +	dev_priv = to_i915(intel_crtc->base.dev);
> > +
> >  	crtc_clock = crtc_state->base.adjusted_mode.crtc_clock;
> > -	cdclk = to_intel_atomic_state(crtc_state->base.state)->cdclk.logical.cdclk;
> > +	max_dotclk = to_intel_atomic_state(crtc_state->base.state)->cdclk.logical.cdclk;
> 
> I actually meant using dev_priv->max_dotclock (or whatever it was
> called). But now that I actually read the code I see that it wouldn't
> work. So this looks fine

There were a few missing words in that reply. ;-)

We could do a max_dotclock_for_cdclk() kind of thing or just track the max
dotclock with the cdclk, but I'm not sure if it improves the situation much.

Ander

> Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> > +
> > +	if (IS_GEMINILAKE(dev_priv))
> > +		max_dotclk *= 2;
> >  
> > -	if (WARN_ON_ONCE(!crtc_clock || cdclk < crtc_clock))
> > +	if (WARN_ON_ONCE(!crtc_clock || max_dotclk < crtc_clock))
> >  		return DRM_PLANE_HELPER_NO_SCALING;
> >  
> >  	/*
> > @@ -13229,7 +13235,8 @@ skl_max_scale(struct intel_crtc *intel_crtc, struct intel_crtc_state *crtc_state
> >  	 *            or
> >  	 *    cdclk/crtc_clock
> >  	 */
> > -	max_scale = min((1 << 16) * 3 - 1, (1 << 8) * ((cdclk << 8) / crtc_clock));
> > +	max_scale = min((1 << 16) * 3 - 1,
> > +			(1 << 8) * ((max_dotclk << 8) / crtc_clock));
> >  
> >  	return max_scale;
> >  }
> > -- 
> > 2.9.3
> 
>