Re: [PATCH 3/9] drm/i915: PSR HSW/BDW: Fix inverted logic at sink main_link_active bit.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jan 13, 2015 at 02:24:54PM +0000, R, Durgadoss wrote:
> Hi Rodrigo,
>
> >-----Original Message-----
> >From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of Rodrigo Vivi
> >Sent: Monday, January 12, 2015 11:45 PM
> >To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> >Cc: Vivi, Rodrigo
> >Subject:  [PATCH 3/9] drm/i915: PSR HSW/BDW: Fix inverted logic at sink main_link_active bit.
> >
> >We have only two possible states with so many names and combinations that
> >might be confusing.
> >
> >1 - Main link active / enabled / stand by / on
> >2 - Main link disabled / off / full off
>
> In this case, I think we should use 'link_active' instead of 'link_standby'
> since 'active' is what is used by the eDP spec and most of our PSR
> macros. But, I believe we can have this as a separate patch.

I haven't read the patches in detail, but consistent naming is imo very
important. Also since this seems to be confusing some extension to the psr
kerneldoc overview comment is imo asked for here.

Rodrigo, can you please update the patches or do a follow up? Either is
fine with me.
-Daniel

>
> For patches 1-3,
> Reviewed-by: Durgadoss R <durgadoss.r@xxxxxxxxx>
>
> Thanks,
> Durga
>
> >
> >Let's start organizing it by fixing a inverted logic when setting the sink bit.
> >
> >Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> >---
> > drivers/gpu/drm/i915/intel_psr.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> >index 3dd8886..0af89db 100644
> >--- a/drivers/gpu/drm/i915/intel_psr.c
> >+++ b/drivers/gpu/drm/i915/intel_psr.c
> >@@ -163,10 +163,10 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
> > /* Enable PSR in sink */
> > if (intel_dp->psr_dpcd[1] & DP_PSR_NO_TRAIN_ON_EXIT || only_standby)
> > drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
> >-   DP_PSR_ENABLE & ~DP_PSR_MAIN_LINK_ACTIVE);
> >+   DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
> > else
> > drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG,
> >-   DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
> >+   DP_PSR_ENABLE & ~DP_PSR_MAIN_LINK_ACTIVE);
> >
> > /* Setup AUX registers */
> > for (i = 0; i < sizeof(aux_msg); i += 4)
> >--
> >2.1.0
> >
> >_______________________________________________
> >Intel-gfx mailing list
> >Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> >http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux