[Intel-gfx] [PATCH] drm/i915: Support pf CRC source on haswell transcoder edp

[Damien Lespiau]

On Thu, May 29, 2014 at 12:41:44PM +0200, Daniel Vetter wrote:
> On Thu, May 29, 2014 at 1:21 AM, Damien Lespiau
> <damien.lespiau at intel.com> wrote:
> > On Thu, May 29, 2014 at 12:27:55AM +0200, Daniel Vetter wrote:
> >> -                     if (IS_HASWELL(dev) && intel_crtc->config.pch_pfit.enabled)
> >> +                     if (IS_HASWELL(dev) && intel_crtc->config.pch_pfit.enabled ||
> >> +                         intel_crtc->config.pch_pfit.force_thru)
> >>                               temp |= TRANS_DDI_EDP_INPUT_A_ONOFF;
> >
> > My gcc warns here, suggesting the addition of (). And indeed, it seems
> > that we want:
> >
> >         if (IS_HASWELL(dev) && (intel_crtc->config.pch_pfit.enabled ||
> >                                 intel_crtc->config.pch_pfit.force_thru))
> >                 temp |= TRANS_DDI_EDP_INPUT_A_ONOFF;
> 
> It doesn't actually matter since force_thru is set only on HSW. We can
> set them whereever we want, and I think actually wrapping it like
> (IS_HSW && pfit.enabled) || pfit.force_thru reads saner. Otoh only HSW
> has this peculiarity.

Either way is fine, but I like mine a bit better because when looking at
these line in isolation (without looking where we set force_thru) it
doesn't smell fishy.

The pf changed on BDW and TRANS_DDI_EDP_INPUT_A_ONOFF is reserved, which
is why I propose that split.
(IS_HSW && pfit.enabled) || pfit.force_thru reads like force_thru could
be set on BDW and we end up setting a reserved value.

-- 
Damien