Re: [PATCH 40/66] drm/i915: Remove spll_refcount for hsw

Daniel Vetter
2014-05-22T19:41:26Z

On Thu, May 22, 2014 at 03:41:25PM -0300, Paulo Zanoni wrote:
> 2014-04-24 18:55 GMT-03:00 Daniel Vetter <daniel.vetter@ffwll.ch>:
> > SPLL would be a reference clock we could potentially share,
> > especially if we want to use the SSC mode. But currently we
> > don't, so let's rip out this complexity for a simpler conversion
> > to the new display pll framework.
>
> I'm really not a fan of this patch, I think it goes in the opposite
> way of where we want to be. We already talked a few times about adding
> sharing support, and I even recently considered adding a
> "i915.edp_use_ssc" module option and ask some bug reporters to test
> them. Also, adding the SPLL support will be another way to prove that
> your "shared DPLL" code is flexible enough to handle the all the
> possibilities generated by the creative minds of the HW developers.
> Can you please include SPLL in your conversion?

Until we actually know that we need it I don't see a reason to do it. And
for a quick debug patch you can always just disable vga on bdw and simply
use the spll for edp without any sharing logic.

But if you wonder who we should integrate SPLL sharing into this framework
it's straightforward:
- Add a possible_dpll_mask to the pipe_config, and pre-fill it in the
encoder->compute_config masks.
- Add spll state the the dpll_hw_state structure, pre-fill that in the
encoder->compute_config hooks (might as well do it right)
- add checks for the possible_dpll_mask in the get_shared_dpll function

Bonus point: We can ditch the ibx special case.

Or you go even simpler: If we need edp SSC _and_ possible VGA we only let
the SPLL run at 2.7GHz.

Then add a 2nd shared_spll to dev_priv which uses the shared dpll
functions and all that logic (might need to frob the interfaces a bit) but
outside of the selection logic (since we always have the same
configuration). Will be a bit more messy since we need to special-case
hw state readout code and a few other places.

Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch