Re: [Intel-gfx] [PATCH 05/13] drm/i915/hdmi: stop caching has_audio in struct intel_hdmi

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

On 4/21/2023 5:17 PM, Jani Nikula wrote:
> Use the information stored in display info.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_display_debugfs.c | 8 +++-----
>   drivers/gpu/drm/i915/display/intel_display_types.h   | 1 -
>   drivers/gpu/drm/i915/display/intel_hdmi.c            | 6 ++----
>   3 files changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> index 1416b82fc8a3..2a4df62692a6 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -252,13 +252,11 @@ static void intel_dp_mst_info(struct seq_file *m,
>   }
>   
>   static void intel_hdmi_info(struct seq_file *m,
> -			    struct intel_connector *intel_connector)
> +			    struct intel_connector *connector)
>   {
> -	struct intel_encoder *intel_encoder = intel_attached_encoder(intel_connector);
> -	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(intel_encoder);
> +	bool has_audio = connector->base.display_info.has_audio;
>   
> -	seq_printf(m, "\taudio support: %s\n",
> -		   str_yes_no(intel_hdmi->has_audio));
> +	seq_printf(m, "\taudio support: %s\n", str_yes_no(has_audio));
>   }
>   
>   static void intel_connector_info(struct seq_file *m,
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 17681a8e745c..d7ff75167f1f 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1549,7 +1549,6 @@ struct intel_hdmi {
>   		int max_tmds_clock;
>   	} dp_dual_mode;
>   	bool has_hdmi_sink;
> -	bool has_audio;
>   	struct intel_connector *attached_connector;
>   	struct cec_notifier *cec_notifier;
>   };
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 8141d5184856..3045bf21ba8f 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -2158,7 +2158,7 @@ static bool intel_hdmi_has_audio(struct intel_encoder *encoder,
>   				 const struct intel_crtc_state *crtc_state,
>   				 const struct drm_connector_state *conn_state)
>   {
> -	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
> +	struct drm_connector *connector = conn_state->connector;
>   	const struct intel_digital_connector_state *intel_conn_state =
>   		to_intel_digital_connector_state(conn_state);
>   
> @@ -2166,7 +2166,7 @@ static bool intel_hdmi_has_audio(struct intel_encoder *encoder,
>   		return false;
>   
>   	if (intel_conn_state->force_audio == HDMI_AUDIO_AUTO)
> -		return intel_hdmi->has_audio;
> +		return connector->display_info.has_audio;
>   	else
>   		return intel_conn_state->force_audio == HDMI_AUDIO_ON;

This now looks similar to DP intel_dp_has_audio, perhaps can be re-used 
in DP as a separate patch.

In any case, the change looks good to me.

Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>


Regards,

Ankit

>   }
> @@ -2360,7 +2360,6 @@ intel_hdmi_unset_edid(struct drm_connector *connector)
>   	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(to_intel_connector(connector));
>   
>   	intel_hdmi->has_hdmi_sink = false;
> -	intel_hdmi->has_audio = false;
>   
>   	intel_hdmi->dp_dual_mode.type = DRM_DP_DUAL_MODE_NONE;
>   	intel_hdmi->dp_dual_mode.max_tmds_clock = 0;
> @@ -2453,7 +2452,6 @@ intel_hdmi_set_edid(struct drm_connector *connector)
>   	/* FIXME: Get rid of drm_edid_raw() */
>   	edid = drm_edid_raw(drm_edid);
>   	if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) {
> -		intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
>   		intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid);
>   
>   		intel_hdmi_dp_dual_mode_detect(connector);