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@xxxxxxxxx> 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
