Re: [PATCH v2 3/4] drm/i915/display: Nuke has_infoframe

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

 



On Fri, 2021-05-21 at 16:27 +0100, Mun, Gwan-gyeong wrote:
> On Fri, 2021-05-14 at 16:22 -0700, José Roberto de Souza wrote:
> > This was only reduntant information has_hdmi_sink can do the same job.
> > set_infoframes() hooks will call intel_write_infoframe() for the
> > supported infoframes types and it will only be enabled if given type
> > is set in crtc_state->infoframes.enable.
> > 
> > While at it also fixing the style of dig_port->set_infoframes() calls.
> > 
> > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
> > ---
> >  drivers/gpu/drm/i915/display/g4x_hdmi.c       | 22 ++++++-------------
> >  drivers/gpu/drm/i915/display/intel_ddi.c      | 17 +++++---------
> >  drivers/gpu/drm/i915/display/intel_display.c  |  6 ++---
> >  .../drm/i915/display/intel_display_types.h    |  3 ---
> >  drivers/gpu/drm/i915/display/intel_dp_mst.c   |  4 ++--
> >  drivers/gpu/drm/i915/display/intel_hdmi.c     | 13 +++++------
> >  6 files changed, 22 insertions(+), 43 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/g4x_hdmi.c
> > b/drivers/gpu/drm/i915/display/g4x_hdmi.c
> > index be352e9f0afc..f35db96e6239 100644
> > --- a/drivers/gpu/drm/i915/display/g4x_hdmi.c
> > +++ b/drivers/gpu/drm/i915/display/g4x_hdmi.c
> > @@ -105,9 +105,6 @@ static void intel_hdmi_get_config(struct
> > intel_encoder *encoder,
> >         pipe_config->infoframes.enable |=
> >                 intel_hdmi_infoframes_enabled(encoder, pipe_config);
> >  
> > -       if (pipe_config->infoframes.enable)
> > -               pipe_config->has_infoframe = true;
> > -
> "pipe_config->infoframes.enable" is set with information about the
> infoframes currently active in the hardware through "pipe_config-
> > infoframes.enable |= intel_hdmi_infoframes_enabled(encoder,
> pipe_config);".
> 
> Therefore, when calling set_infoframes() semantically, the
> has_infoframe information set by "if (pipe_config->infoframes.enable)
> pipe_config->has_infoframe = true;" is more clear.

That don't work because the functions that will check if a infoframe is needed and set pipe_config->infoframes.enable depends on pipe_config-
>has_infoframe/crtc_state->has_hdmi_sink.
That is probably because DVI ports don't support infoframes but in i915 are handle very similar to HDMI.

> 
> >         if (tmp & HDMI_AUDIO_ENABLE)
> >                 pipe_config->has_audio = true;
> >  
> > @@ -343,9 +340,7 @@ static void intel_disable_hdmi(struct
> > intel_atomic_state *state,
> >                 intel_set_pch_fifo_underrun_reporting(dev_priv, PIPE_A,
> > true);
> >         }
> >  
> > -       dig_port->set_infoframes(encoder,
> > -                                      false,
> > -                                      old_crtc_state, old_conn_state);
> > +       dig_port->set_infoframes(encoder, false, old_crtc_state,
> > old_conn_state);
> >  
> >         intel_dp_dual_mode_set_tmds_output(intel_hdmi, false);
> >  }
> > @@ -390,9 +385,8 @@ static void intel_hdmi_pre_enable(struct
> > intel_atomic_state *state,
> >  
> >         intel_hdmi_prepare(encoder, pipe_config);
> >  
> > -       dig_port->set_infoframes(encoder,
> > -                                      pipe_config->has_infoframe,
> > -                                      pipe_config, conn_state);
> > +       dig_port->set_infoframes(encoder, pipe_config->has_hdmi_sink,
> > +                                pipe_config, conn_state);
> >  }
> >  
> >  static void vlv_hdmi_pre_enable(struct intel_atomic_state *state,
> > @@ -410,9 +404,8 @@ static void vlv_hdmi_pre_enable(struct
> > intel_atomic_state *state,
> >                                  0x2b245f5f, 0x00002000,
> >                                  0x5578b83a, 0x2b247878);
> >  
> > -       dig_port->set_infoframes(encoder,
> > -                             pipe_config->has_infoframe,
> > -                             pipe_config, conn_state);
> > +       dig_port->set_infoframes(encoder, pipe_config->has_hdmi_sink,
> > +                                pipe_config, conn_state);
> >  
> >         g4x_enable_hdmi(state, encoder, pipe_config, conn_state);
> >  
> > @@ -487,9 +480,8 @@ static void chv_hdmi_pre_enable(struct
> > intel_atomic_state *state,
> >         /* Use 800mV-0dB */
> >         chv_set_phy_signal_level(encoder, pipe_config, 128, 102,
> > false);
> >  
> > -       dig_port->set_infoframes(encoder,
> > -                             pipe_config->has_infoframe,
> > -                             pipe_config, conn_state);
> > +       dig_port->set_infoframes(encoder, pipe_config->has_hdmi_sink,
> > +                                pipe_config, conn_state);
> >  
> >         g4x_enable_hdmi(state, encoder, pipe_config, conn_state);
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index b7a2fce684c9..5bc5528f3091 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -2722,9 +2722,8 @@ static void intel_ddi_pre_enable_hdmi(struct
> > intel_atomic_state *state,
> >  
> >         intel_ddi_enable_pipe_clock(encoder, crtc_state);
> >  
> > -       dig_port->set_infoframes(encoder,
> > -                                crtc_state->has_infoframe,
> > -                                crtc_state, conn_state);
> > +       dig_port->set_infoframes(encoder, crtc_state->has_hdmi_sink,
> > crtc_state,
> > +                                conn_state);
> >  }
> >  
> >  static void intel_ddi_pre_enable(struct intel_atomic_state *state,
> > @@ -2765,9 +2764,8 @@ static void intel_ddi_pre_enable(struct
> > intel_atomic_state *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,
> > -                                                crtc_state-
> > > has_infoframe,
> > -                                                crtc_state,
> > conn_state);
> > +                       dig_port->set_infoframes(encoder, true,
> > crtc_state,
> > +                                                conn_state);
> >         }
> >  }
> >  
> > @@ -2813,8 +2811,8 @@ static void intel_ddi_post_disable_dp(struct
> > intel_atomic_state *state,
> >         enum phy phy = intel_port_to_phy(dev_priv, encoder->port);
> >  
> >         if (!is_mst)
> > -               intel_dp_set_infoframes(encoder, false,
> > -                                       old_crtc_state,
> > old_conn_state);
> > +               intel_dp_set_infoframes(encoder, false, old_crtc_state,
> > +                                       old_conn_state);
> >  
> >         /*
> >          * Power down sink before disabling the port, otherwise we end
> > @@ -3569,9 +3567,6 @@ static void intel_ddi_read_func_ctl(struct
> > intel_encoder *encoder,
> >                 pipe_config->infoframes.enable |=
> >                         intel_hdmi_infoframes_enabled(encoder,
> > pipe_config);
> >  
> > -               if (pipe_config->infoframes.enable)
> > -                       pipe_config->has_infoframe = true;
> > -
> >                 if (temp & TRANS_DDI_HDMI_SCRAMBLING)
> >                         pipe_config->hdmi_scrambling = true;
> >                 if (temp & TRANS_DDI_HIGH_TMDS_CHAR_RATE)
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index ebac1bd5cfe5..5d68b253bdfe 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -7594,9 +7594,8 @@ static void intel_dump_pipe_config(const struct
> > intel_crtc_state *pipe_config,
> >         }
> >  
> >         drm_dbg_kms(&dev_priv->drm,
> > -                   "audio: %i, infoframes: %i, infoframes enabled:
> > 0x%x\n",
> > -                   pipe_config->has_audio, pipe_config->has_infoframe,
> > -                   pipe_config->infoframes.enable);
> > +                   "audio: %i, infoframes enabled: 0x%x\n",
> > +                   pipe_config->has_audio, pipe_config-
> > > infoframes.enable);
> >  
> >         if (pipe_config->infoframes.enable &
> >            
> > intel_hdmi_infoframe_enable(HDMI_PACKET_TYPE_GENERAL_CONTROL))
> > @@ -8508,7 +8507,6 @@ intel_pipe_config_compare(const struct
> > intel_crtc_state *current_config,
> >  
> >         PIPE_CONF_CHECK_BOOL(hdmi_scrambling);
> >         PIPE_CONF_CHECK_BOOL(hdmi_high_tmds_clock_ratio);
> > -       PIPE_CONF_CHECK_BOOL(has_infoframe);
> >         /* FIXME do the readout properly and get rid of this quirk */
> >         if (!PIPE_CONF_QUIRK(PIPE_CONFIG_QUIRK_BIGJOINER_SLAVE))
> >                 PIPE_CONF_CHECK_BOOL(fec_enable);
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 9c0adfc60c6f..669c5d8a2131 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -952,9 +952,6 @@ struct intel_crtc_state {
> >          * between pch encoders and cpu encoders. */
> >         bool has_pch_encoder;
> >  
> > -       /* Are we sending infoframes on the attached port */
> > -       bool has_infoframe;
> > -
> >         /* CPU Transcoder for the pipe. Currently this can only differ
> > from the
> >          * pipe on Haswell and later (where we have a special eDP
> > transcoder)
> >          * and Broxton (where we have special DSI transcoders). */
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > index 332d2f9fda5c..1eb54f8ed51a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> > @@ -435,8 +435,8 @@ static void intel_mst_post_disable_dp(struct
> > intel_atomic_state *state,
> >          * the transcoder clock select is set to none.
> >          */
> >         if (last_mst_stream)
> > -               intel_dp_set_infoframes(&dig_port->base, false,
> > -                                       old_crtc_state, NULL);
> > +               intel_dp_set_infoframes(&dig_port->base, false,
> > old_crtc_state,
> > +                                       old_conn_state);
> >         /*
> >          * From TGL spec: "If multi-stream slave transcoder: Configure
> >          * Transcoder Clock Select to direct no clock to the
> > transcoder"
> > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
> > b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > index 4a1b2d863b0c..4b970587067d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> > @@ -712,7 +712,7 @@ intel_hdmi_compute_avi_infoframe(struct
> > intel_encoder *encoder,
> >         struct drm_connector *connector = conn_state->connector;
> >         int ret;
> >  
> > -       if (!crtc_state->has_infoframe)
> > +       if (!crtc_state->has_hdmi_sink)
> >                 return true;
> >  
> >         crtc_state->infoframes.enable |=
> > @@ -766,7 +766,7 @@ intel_hdmi_compute_spd_infoframe(struct
> > intel_encoder *encoder,
> >         struct hdmi_spd_infoframe *frame = &crtc_state-
> > > infoframes.spd.spd;
> >         int ret;
> >  
> > -       if (!crtc_state->has_infoframe)
> > +       if (!crtc_state->has_hdmi_sink)
> >                 return true;
> >  
> >         crtc_state->infoframes.enable |=
> > @@ -796,7 +796,7 @@ intel_hdmi_compute_hdmi_infoframe(struct
> > intel_encoder *encoder,
> >                 &conn_state->connector->display_info;
> >         int ret;
> >  
> > -       if (!crtc_state->has_infoframe || !info->has_hdmi_infoframe)
> > +       if (!crtc_state->has_hdmi_sink || !info->has_hdmi_infoframe)
> >                 return true;
> >  
> >         crtc_state->infoframes.enable |=
> > @@ -827,7 +827,7 @@ intel_hdmi_compute_drm_infoframe(struct
> > intel_encoder *encoder,
> >         if (DISPLAY_VER(dev_priv) < 10)
> >                 return true;
> >  
> > -       if (!crtc_state->has_infoframe)
> > +       if (!crtc_state->has_hdmi_sink)
> >                 return true;
> >  
> >         if (!conn_state->hdr_output_metadata)
> > @@ -1018,7 +1018,7 @@ static void
> > intel_hdmi_compute_gcp_infoframe(struct intel_encoder *encoder,
> >  {
> >         struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >  
> > -       if (IS_G4X(dev_priv) || !crtc_state->has_infoframe)
> > +       if (IS_G4X(dev_priv) || !crtc_state->has_hdmi_sink)
> >                 return;
> >  
> >         crtc_state->infoframes.enable |=
> > @@ -2172,9 +2172,6 @@ int intel_hdmi_compute_config(struct
> > intel_encoder *encoder,
> >         pipe_config->has_hdmi_sink = intel_has_hdmi_sink(intel_hdmi,
> >                                                          conn_state);
> >  
> > -       if (pipe_config->has_hdmi_sink)
> > -               pipe_config->has_infoframe = true;
> > -
> >         if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
> >                 pipe_config->pixel_multiplier = 2;
> >  
> 

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux