[Intel-gfx] [PATCH v2 06/20] drm/i915: Allow fuzzy matching in pipe_config_compare.

[Daniel Vetter]

On Wed, Jul 08, 2015 at 11:18:27AM +0200, Maarten Lankhorst wrote:
> Op 08-07-15 om 11:09 schreef Daniel Vetter:
> > On Wed, Jul 08, 2015 at 10:38:33AM +0200, Maarten Lankhorst wrote:
> >> Op 07-07-15 om 12:11 schreef Daniel Vetter:
> >>> On Tue, Jul 07, 2015 at 09:08:17AM +0200, Maarten Lankhorst wrote:
> >>>> @@ -12392,27 +12379,124 @@ static bool intel_fuzzy_clock_check(int clock1, int clock2)
> >>>>  			    base.head) \
> >>>>  		if (mask & (1 <<(intel_crtc)->pipe))
> >>>>  
> >>>> +
> >>>> +static bool
> >>>> +intel_compare_m_n(unsigned int m, unsigned int n,
> >>>> +		  unsigned int m2, unsigned int n2)
> >>>> +{
> >>>> +	if (m == m2 && n == n2)
> >>>> +		return true;
> >>>> +
> >>>> +	if (!m || !n || !m2 || !n2)
> >>>> +		return false;
> >>>> +
> >>>> +	if (m > m2) {
> >>>> +		while (m > m2) {
> >>> I think we need to make sure we don't reduce precision by eating low bits,
> >>> i.e.
> >>> 		while (m > m2 && !(m & 1) && !(n & 1)) {
> >> Could be useful, lets hope everyone rounds in the same way. :)
> >>>> +			m >>= 1;
> >>>> +			n >>= 1;
> >>>> +		}
> >>>> +	} else if (m < m2) {
> >>>> +		while (m < m2) {
> >>>> +			m2 >>= 1;
> >>>> +			n2 >>= 1;
> >>>> +		}
> >>>> +	}
> >>>> +
> >>>> +	return m == m2 && n == n2;
> >>>> +}
> >>>> +
> >>>> +static bool
> >>>> +intel_compare_link_m_n(const struct intel_link_m_n *m_n,
> >>>> +		       const struct intel_link_m_n *m2_n2)
> >>> I think these need to take adjust as an agurment and check for exact match
> >>> (not just matching ratio), like before.
> >> I just implemented it, then remembered why I didn't.
> >>
> >> It will cause a failure in intel_modeset_check_state.
> > That shouldn't happen. I guess the bug is that adjust doesn't live up to
> > it's name and doesn't copy the current value over to the new config if
> > they're compatible?
> It's a compare function, I didn't want to do adjustments in it  and leaving it as a multiple is harmless.
> In case of gen >= 8 it might even fix itself through intel_dp_set_drrs_state.

Well pass it the bool adjust like for config_compare. I think in general
(i.e. when it's not too much work, which isn't the case here) doing exact
matches for hw/sw state checks is the right approach.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch