> -----Original Message----- > From: Deak, Imre <imre.deak@xxxxxxxxx> > Sent: Tuesday, July 23, 2024 5:26 PM > To: Kandpal, Suraj <suraj.kandpal@xxxxxxxxx> > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH 04/14] drm/i915/ddi: For an active output call the DP > encoder sync_state() only for DP > > On Tue, Jul 23, 2024 at 11:28:33AM +0300, Kandpal, Suraj wrote: > > > > > > > -----Original Message----- > > > From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf > > > Of Imre Deak > > > Sent: Monday, July 22, 2024 10:25 PM > > > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > > Subject: [PATCH 04/14] drm/i915/ddi: For an active output call the > > > DP encoder sync_state() only for DP > > > > > > If the DDI encoder output is enabled in HDMI mode there is no point > > > in calling intel_dp_sync_state(), as in that case the DPCD > > > initialization will fail - as expected - with AUX timeouts. Prevent calling > the hook in this case. > > > > > > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx> > > > --- > > > drivers/gpu/drm/i915/display/intel_ddi.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > > > b/drivers/gpu/drm/i915/display/intel_ddi.c > > > index a07aca96e5517..11ee4406dce8f 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > > > @@ -4172,7 +4172,8 @@ static void intel_ddi_sync_state(struct > > > intel_encoder *encoder, > > > intel_tc_port_sanitize_mode(enc_to_dig_port(encoder), > > > crtc_state); > > > > > > - if (intel_encoder_is_dp(encoder)) > > > + if ((crtc_state && intel_crtc_has_dp_encoder(crtc_state)) || > > > + (!crtc_state && intel_encoder_is_dp(encoder))) > > > > So we are trying to avoid calling intel_dp_sync_state incase > > intel_encoder_is_dp returns INTEL_OUTPUT_DDI in that case why are we > > still keeping the check intel_encoder_is_dp when crtc_state is not > > present. > > intel_encoder_is_dp() returns true if a DP connector is registered using this > DDI encoder. If the output is disabled (so crtc_state==NULL) that's the only > way to determine if the encoder may be used for DP (unless an HDMI > connector is also registered using this same encoder and that's what is > actually used on the given platform). In case the output is enabled the > DP/HDMI mode in crtc_state gives the same answer, but by checking that > instead we avoid calling the DP specific hook if the encoder is used by HDMI. > Ohkay got it In that case LGTM, Reviewed-by: Suraj Kandpal <suraj.kandpal@xxxxxxxxx> > > Regards, > > Suraj Kandpal > > > > > intel_dp_sync_state(encoder, crtc_state); } > > > > > > -- > > > 2.44.2 > >
