On 06/15/2015 02:09 PM, Damien Lespiau wrote:
On Mon, Jun 15, 2015 at 01:40:24PM +0100, Tvrtko Ursulin wrote:
On 06/15/2015 01:14 PM, Damien Lespiau wrote:
On Mon, Jun 15, 2015 at 01:54:40PM +0200, Daniel Vetter wrote:
On Wed, Jun 03, 2015 at 03:45:08PM +0300, Mika Kahola wrote:
From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Rather than reading out the current cdclk value use the cached value we
have tucked away in dev_priv.
v2: Rebased to the latest
v3: Rebased to the latest
v4: Fix for patch style problems
Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Signed-off-by: Mika Kahola <mika.kahola@xxxxxxxxx>
This patch needs to be extended to also cover the recently added
skl_max_scale. Tvrtko has recently written a patch to add some checks to
that code too, would be good to resurrect that too. Chandra can help with
any questions wrt the skl scaler code.
Not quite I'm afraid. The CDCLK used in skl_max_scale() has to be part
of the atomic state, even bumping CDCLK if possible/needed.
If you use the cached cdclk in skl_max_scale(), it won't do the right
thing when CDCLK is off (ie cached frew is the fallback 24Mhz ref clock)
and we try to do the first modeset before waking up the display.
I filed a bug about it already to track it:
https://bugs.freedesktop.org/show_bug.cgi?id=90874
I know nothing about these specific clocks, but FWIW, my patch was only
about enabling new platforms - making skl_max_scale more robust in cases
where clock querying does not yet work correctly.
My reasoning was based on a comment from Ville that one of those two clocks
must never be lower than the other.
So it sounded reasonable to ignore such cases ie. assume no scaling is
possible and allow a normal (unscaled) modeset to succeed rather than fail
it and display nothing.
So to be more specific, I believe this is because we detect CDCLK as
being "disabled" or on the ref clock in simulation?
Probably a reference clock. It definitely wasn't zero since
skl_max_scale already handles that. But I forgot the exact details.
Generally speaking, it's questionable if we want to work around such
limitations in the code like that, I'd rather go for defaulting to max
CDCLK in simulation.
In this particular case, we really shouldn't get cdclk < crtc_clock at
this point, I'd expect the cdclk we use (probably part of the atomic
state) to be bumped to cover crtc_clock prior to plane checks (See
Marteen's [PATCH v3 19/19] drm/i915: Make cdclk part of the atomic
state.), I guess we could add a WARN_ON(cdclk < crtc_clock) in
skl_max_scale() to ensure that's indeed the case?
WARN_ON sounds fine to me. For the other considerations - you're the
expert. :)
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
