| Message ID | 20210514232247.144542-4-jose.souza@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/4] drm/i915/display: Fix fastsets involving PSR | expand |
> -----Original Message----- > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of José > Roberto de Souza > Sent: Friday, May 14, 2021 4:23 PM > To: intel-gfx@lists.freedesktop.org > Subject: [Intel-gfx] [PATCH v2 4/4] drm/i915/display: Drop FIXME about turn > off infoframes > > intel_dp_set_infoframes() call in intel_ddi_post_disable_dp() will take care to > disable all enabled infoframes. > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > --- > drivers/gpu/drm/i915/display/intel_ddi.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > b/drivers/gpu/drm/i915/display/intel_ddi.c > index 5bc5528f3091..d3bc5a1a936a 100644 > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > @@ -2762,7 +2762,6 @@ static void intel_ddi_pre_enable(struct > intel_atomic_state *state, > conn_state); > > /* FIXME precompute everything properly */ > - /* FIXME how do we turn infoframes off again? */ > if (dig_port->lspcon.active && dig_port->dp.has_hdmi_sink) > dig_port->set_infoframes(encoder, true, crtc_state, > conn_state); > -- > 2.31.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, May 14, 2021 at 04:22:47PM -0700, José Roberto de Souza wrote: > intel_dp_set_infoframes() call in intel_ddi_post_disable_dp() will > take care to disable all enabled infoframes. > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > --- > drivers/gpu/drm/i915/display/intel_ddi.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c > index 5bc5528f3091..d3bc5a1a936a 100644 > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > @@ -2762,7 +2762,6 @@ static void intel_ddi_pre_enable(struct intel_atomic_state *state, > conn_state); > > /* FIXME precompute everything properly */ > - /* FIXME how do we turn infoframes off again? */ The FIXME was there for LSPCON and shouldn't have been removed. No one has yet figured out how to do this. > if (dig_port->lspcon.active && dig_port->dp.has_hdmi_sink) > dig_port->set_infoframes(encoder, true, crtc_state, > conn_state); > -- > 2.31.1
On Tue, 2021-06-08 at 10:26 +0300, Ville Syrjälä wrote: > On Fri, May 14, 2021 at 04:22:47PM -0700, José Roberto de Souza wrote: > > intel_dp_set_infoframes() call in intel_ddi_post_disable_dp() will > > take care to disable all enabled infoframes. > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_ddi.c | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c > > index 5bc5528f3091..d3bc5a1a936a 100644 > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > > @@ -2762,7 +2762,6 @@ static void intel_ddi_pre_enable(struct intel_atomic_state *state, > > conn_state); > > > > /* FIXME precompute everything properly */ > > - /* FIXME how do we turn infoframes off again? */ > > The FIXME was there for LSPCON and shouldn't have been removed. > No one has yet figured out how to do this. intel_ddi_post_disable_dp()->intel_dp_set_infoframes() will be executed for LSPCON, or am I missing something? > > > if (dig_port->lspcon.active && dig_port->dp.has_hdmi_sink) > > dig_port->set_infoframes(encoder, true, crtc_state, > > conn_state); > > -- > > 2.31.1 >
On Wed, Jun 09, 2021 at 07:25:36PM +0000, Souza, Jose wrote: > On Tue, 2021-06-08 at 10:26 +0300, Ville Syrjälä wrote: > > On Fri, May 14, 2021 at 04:22:47PM -0700, José Roberto de Souza wrote: > > > intel_dp_set_infoframes() call in intel_ddi_post_disable_dp() will > > > take care to disable all enabled infoframes. > > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > > --- > > > drivers/gpu/drm/i915/display/intel_ddi.c | 1 - > > > 1 file changed, 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c > > > index 5bc5528f3091..d3bc5a1a936a 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > > > @@ -2762,7 +2762,6 @@ static void intel_ddi_pre_enable(struct intel_atomic_state *state, > > > conn_state); > > > > > > /* FIXME precompute everything properly */ > > > - /* FIXME how do we turn infoframes off again? */ > > > > The FIXME was there for LSPCON and shouldn't have been removed. > > No one has yet figured out how to do this. > > intel_ddi_post_disable_dp()->intel_dp_set_infoframes() will be executed for LSPCON, or am I missing something? LSPCON generates its own infoframes, so turning off the video DIP does diddly squat.
On Thu, 2021-06-10 at 15:18 +0300, Ville Syrjälä wrote: > On Wed, Jun 09, 2021 at 07:25:36PM +0000, Souza, Jose wrote: > > On Tue, 2021-06-08 at 10:26 +0300, Ville Syrjälä wrote: > > > On Fri, May 14, 2021 at 04:22:47PM -0700, José Roberto de Souza wrote: > > > > intel_dp_set_infoframes() call in intel_ddi_post_disable_dp() will > > > > take care to disable all enabled infoframes. > > > > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > > > --- > > > > drivers/gpu/drm/i915/display/intel_ddi.c | 1 - > > > > 1 file changed, 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c > > > > index 5bc5528f3091..d3bc5a1a936a 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > > > > @@ -2762,7 +2762,6 @@ static void intel_ddi_pre_enable(struct intel_atomic_state *state, > > > > conn_state); > > > > > > > > /* FIXME precompute everything properly */ > > > > - /* FIXME how do we turn infoframes off again? */ > > > > > > The FIXME was there for LSPCON and shouldn't have been removed. > > > No one has yet figured out how to do this. > > > > intel_ddi_post_disable_dp()->intel_dp_set_infoframes() will be executed for LSPCON, or am I missing something? > > LSPCON generates its own infoframes, so turning off the video DIP does > diddly squat. > It would not be the lspcon job to do so when DP -> lspcon video is off? Anyways will be sending the revert in a few minutes, please review that if we really need this fixme back.
On Thu, Jun 10, 2021 at 05:44:12PM +0000, Souza, Jose wrote: > On Thu, 2021-06-10 at 15:18 +0300, Ville Syrjälä wrote: > > On Wed, Jun 09, 2021 at 07:25:36PM +0000, Souza, Jose wrote: > > > On Tue, 2021-06-08 at 10:26 +0300, Ville Syrjälä wrote: > > > > On Fri, May 14, 2021 at 04:22:47PM -0700, José Roberto de Souza wrote: > > > > > intel_dp_set_infoframes() call in intel_ddi_post_disable_dp() will > > > > > take care to disable all enabled infoframes. > > > > > > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > > > > --- > > > > > drivers/gpu/drm/i915/display/intel_ddi.c | 1 - > > > > > 1 file changed, 1 deletion(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c > > > > > index 5bc5528f3091..d3bc5a1a936a 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_ddi.c > > > > > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c > > > > > @@ -2762,7 +2762,6 @@ static void intel_ddi_pre_enable(struct intel_atomic_state *state, > > > > > conn_state); > > > > > > > > > > /* FIXME precompute everything properly */ > > > > > - /* FIXME how do we turn infoframes off again? */ > > > > > > > > The FIXME was there for LSPCON and shouldn't have been removed. > > > > No one has yet figured out how to do this. > > > > > > intel_ddi_post_disable_dp()->intel_dp_set_infoframes() will be executed for LSPCON, or am I missing something? > > > > LSPCON generates its own infoframes, so turning off the video DIP does > > diddly squat. > > > > It would not be the lspcon job to do so when DP -> lspcon video is off? LSPCON is a blackbox and no one knows what it actually does. Which is pretty much the point of the FIXME. > Anyways will be sending the revert in a few minutes, please review that if we really need this fixme back.
diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c index 5bc5528f3091..d3bc5a1a936a 100644 --- a/drivers/gpu/drm/i915/display/intel_ddi.c +++ b/drivers/gpu/drm/i915/display/intel_ddi.c @@ -2762,7 +2762,6 @@ static void intel_ddi_pre_enable(struct intel_atomic_state *state, conn_state); /* FIXME precompute everything properly */ - /* FIXME how do we turn infoframes off again? */ if (dig_port->lspcon.active && dig_port->dp.has_hdmi_sink) dig_port->set_infoframes(encoder, true, crtc_state, conn_state);
intel_dp_set_infoframes() call in intel_ddi_post_disable_dp() will take care to disable all enabled infoframes. Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> --- drivers/gpu/drm/i915/display/intel_ddi.c | 1 - 1 file changed, 1 deletion(-)