Re: [Freedreno] [PATCH] drm/msm/dp: do not notify audio subsystem if sink doesn't support audio

[abhinavk@codeaurora.org]

Hi Stephen

On 2020-11-02 15:06, Stephen Boyd wrote:
> Quoting abhinavk@codeaurora.org (2020-11-02 14:43:33)
>> Hi Stephen
>> 
>> Thanks for the review.
>> 
>> On 2020-11-02 13:19, Stephen Boyd wrote:
>> > Quoting Abhinav Kumar (2020-10-29 13:55:09)
>> >> For sinks that do not support audio, there is no need to notify
>> >> audio subsystem of the connection event.
>> >>
>> >> This will make sure that audio routes only to the primary display
>> >> when connected to such sinks.
>> >>
>> >
>> > Does this need a Fixes tag? Or it's just an optimization patch?
>> This is an unhandled corner case ( VGA dongle ) for DP audio and will
>> make
>> sure we do not switch audio output from primary to external when
>> connected to
>> a sink which does not support audio.
>> I thought of adding a fixes tag pointing to
>> https://patchwork.freedesktop.org/patch/390236/.
>> But at the same time, thought this can go in as a standlone patch as
>> well.
>> If you think its required, I will add the fixes tag pointing to the 
>> base
>> audio patch.
> 
> Yeah Fixes is nice so we know what is fixed.
Ok sure, will include it.
> 
>> >
>> >> Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org>
>> >> ---
>> >>  drivers/gpu/drm/msm/dp/dp_display.c | 12 ++++++++++--
>> >>  1 file changed, 10 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
>> >> b/drivers/gpu/drm/msm/dp/dp_display.c
>> >> index 4a5735564be2..d970980b0ca5 100644
>> >> --- a/drivers/gpu/drm/msm/dp/dp_display.c
>> >> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
>> >> @@ -555,8 +555,16 @@ static int dp_connect_pending_timeout(struct
>> >> dp_display_private *dp, u32 data)
>> >>  static void dp_display_handle_plugged_change(struct msm_dp
>> >> *dp_display,
>> >>                 bool plugged)
>> >>  {
>> >> -       if (dp_display->plugged_cb && dp_display->codec_dev)
>> >> -               dp_display->plugged_cb(dp_display->codec_dev,
>> >> plugged);
>> >> +       struct dp_display_private *dp;
>> >> +
>> >> +       dp = container_of(g_dp_display,
>> >
>> > What is g_dp_display? I guess this doesn't compile?
>> g_dp_display is the global dp_display pointer in the dp_display.c 
>> file.
>> It does compile.
> 
> Oh gross. Can we not use it if dp_display points to it?
Ok sure, will get it from dp_display