Re: [PATCH 1/3] drm/i915: Dont enable hpd for eDP

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Aug 13, 2015 at 09:35:21AM +0530, Sivakumar Thulasimani wrote:
> 
> 
> On 8/12/2015 6:26 PM, Daniel Vetter wrote:
> > On Mon, Aug 10, 2015 at 05:51:48PM +0530, Sivakumar Thulasimani wrote:
> >>
> >> On 8/10/2015 5:44 PM, Jani Nikula wrote:
> >>> On Mon, 10 Aug 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
> >>>> On 8/10/2015 5:07 PM, Jani Nikula wrote:
> >>>>> On Mon, 10 Aug 2015, Sivakumar Thulasimani <sivakumar.thulasimani@intel.com> wrote:
> >>>>>> Reviewed-by: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
> >>>>>>
> >>>>>> On 8/10/2015 10:35 AM, Sonika Jindal wrote:
> >>>>>>> With HPD support added for all ports including PORT_A, setting hpd_pin will
> >>>>>>> result in enabling of hpd to edp as well. There is no need to enable HPD on
> >>>>>>> PORT_A hence this patch removes hpd_pin update for PORT_A, where edp will
> >>>>>>> be connected. it can be added back when required
> >>>>> What? You can't just go ahead and remove HPD from eDP sinks.
> >>>>>
> >>>>> BR,
> >>>>> Jani.
> >>>> Nope, we are not removing HPD for edp sinks, it was never there in the
> >>>> first place. It was
> >>>> enabled for CHV (even there by mistake since PORT B/C was both DP and
> >>>> eDP) but it was
> >>>> never there for any other plaforms nor is it used for any purpose (PSR
> >>>> must use it, but i
> >>>> dont see code for it as well).
> >>> Are you saying there's no HPD enabled in our *hardware* for eDP? Or
> >>> driver?
> >>>
> >>> My point is, is this patch making it harder to enable eDP hpd handling
> >>> (e.g. for PSR or DP link re-training) in the future? We currently take
> >>> it into account in a few places, and if we start removing that, it will
> >>> be a loss of effort to first remove and then add it back.
> >>>
> >>> BR,
> >>> Jani.
> >> i was referring to our driver only.
> >>
> >> Our VLV/CHV code already receives hpd for every pps on and off which is
> >> later ignored. if we dont disable HPD on eDPs this behavior will be extended
> >> for all platforms which i feel is too costly to keep enabled when there is
> >> no
> >> purpose for it right now.
> > don't optimize code because you "feel it's costly", only do it when you
> > have hard numbers. One interrupt per pps on or off transition won't be
> > measurable at all.
> > -Daniel
> let me rephrase my concern then :)
> a) HPD was never enabled before this patch for edp

It was for port!=A, which means all VLV/CHV + some other machines with
eDP on port D.

> b) this patch series will enable hpd for edp
> so why should we allow hpd for edp when no one is using it and will 
> cause problems
> unless ignored explicitly ?

It's not clear it would cause problems. My ILK still seems happy now that
I enabled port A HPD on it [1]. As are the VLV/CHV machines. On BSW we
had problems but that turned out to be misconfigured pullups on the HPD
pins. Before that got fixed there was a lot of noise on the line
whenever the panel was off, and that spurred me to write the patch
series to keep the HPD disabled while the port is disabled [2].

[1] http://lists.freedesktop.org/archives/intel-gfx/2015-August/073559.html
[2] http://lists.freedesktop.org/archives/intel-gfx/2015-January/058173.html

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx