[PATCH v2 04/14] drm/msm/hdmi: set infoframes on all pre_enable calls

Dmitry Baryshkov posted 14 patches 1 year, 8 months ago
There is a newer version of this series
[PATCH v2 04/14] drm/msm/hdmi: set infoframes on all pre_enable calls
Posted by Dmitry Baryshkov 1 year, 8 months ago
In consequent modeset calls, the atomic_pre_enable() will be called
several times without calling atomic_post_disable() inbetween. Thus
iframes will not be updated for the next mode. Fix this by setting the
iframe outside of the !power_on check.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
index 3c6121c57b01..fb99328107dd 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
@@ -133,10 +133,11 @@ static void msm_hdmi_bridge_atomic_pre_enable(struct drm_bridge *bridge,
 		msm_hdmi_phy_resource_enable(phy);
 		msm_hdmi_power_on(bridge);
 		hdmi->power_on = true;
-		if (hdmi->hdmi_mode) {
-			msm_hdmi_config_avi_infoframe(hdmi);
-			msm_hdmi_audio_update(hdmi);
-		}
+	}
+
+	if (hdmi->hdmi_mode) {
+		msm_hdmi_config_avi_infoframe(hdmi);
+		msm_hdmi_audio_update(hdmi);
 	}
 
 	msm_hdmi_phy_powerup(phy, hdmi->pixclock);

-- 
2.39.2
Re: [PATCH v2 04/14] drm/msm/hdmi: set infoframes on all pre_enable calls
Posted by Jessica Zhang 1 year, 8 months ago

On 5/22/2024 3:50 AM, Dmitry Baryshkov wrote:
> In consequent modeset calls, the atomic_pre_enable() will be called
> several times without calling atomic_post_disable() inbetween. Thus

Hi Dmitry,

Just wondering, where are you seeing multiple pre_enable() calls without 
a post_disable() happening?

IIRC, the msm commit_tail always calls commit_modeset_disables() before 
the commit_modeset_enables(). Also, doesn't the pre_enable() and 
post_disable() only happen once for bringing up/down the bridge?

Thanks,

Jessica Zhang

> iframes will not be updated for the next mode. Fix this by setting the
> iframe outside of the !power_on check.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> index 3c6121c57b01..fb99328107dd 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> @@ -133,10 +133,11 @@ static void msm_hdmi_bridge_atomic_pre_enable(struct drm_bridge *bridge,
>   		msm_hdmi_phy_resource_enable(phy);
>   		msm_hdmi_power_on(bridge);
>   		hdmi->power_on = true;
> -		if (hdmi->hdmi_mode) {
> -			msm_hdmi_config_avi_infoframe(hdmi);
> -			msm_hdmi_audio_update(hdmi);
> -		}
> +	}
> +
> +	if (hdmi->hdmi_mode) {
> +		msm_hdmi_config_avi_infoframe(hdmi);
> +		msm_hdmi_audio_update(hdmi);
>   	}
>   
>   	msm_hdmi_phy_powerup(phy, hdmi->pixclock);
> 
> -- 
> 2.39.2
>
Re: [PATCH v2 04/14] drm/msm/hdmi: set infoframes on all pre_enable calls
Posted by Dmitry Baryshkov 1 year, 8 months ago
On Wed, 12 Jun 2024 at 03:04, Jessica Zhang <quic_jesszhan@quicinc.com> wrote:
>
>
>
> On 5/22/2024 3:50 AM, Dmitry Baryshkov wrote:
> > In consequent modeset calls, the atomic_pre_enable() will be called
> > several times without calling atomic_post_disable() inbetween. Thus
>
> Hi Dmitry,
>
> Just wondering, where are you seeing multiple pre_enable() calls without
> a post_disable() happening?

I think that was with me hacking around, so the commit is indeed incorrect.

>
> IIRC, the msm commit_tail always calls commit_modeset_disables() before
> the commit_modeset_enables(). Also, doesn't the pre_enable() and
> post_disable() only happen once for bringing up/down the bridge?

I don't know if you mean it, but they are called each time the output
gets disabled and then enabled again, e.g. during modeswitch.

Last, but not least, I'm planning to land the HDMI rework ([1]) once
the drm-misc-next is merged into msm-next ([2]). This makes this
commit obsolete.

[1] https://lore.kernel.org/dri-devel/20240607-bridge-hdmi-connector-v5-0-ab384e6021af@linaro.org/
[2] https://gitlab.freedesktop.org/drm/msm/-/merge_requests/118


>
> Thanks,
>
> Jessica Zhang
>
> > iframes will not be updated for the next mode. Fix this by setting the
> > iframe outside of the !power_on check.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >   drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 9 +++++----
> >   1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> > index 3c6121c57b01..fb99328107dd 100644
> > --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> > +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> > @@ -133,10 +133,11 @@ static void msm_hdmi_bridge_atomic_pre_enable(struct drm_bridge *bridge,
> >               msm_hdmi_phy_resource_enable(phy);
> >               msm_hdmi_power_on(bridge);
> >               hdmi->power_on = true;
> > -             if (hdmi->hdmi_mode) {
> > -                     msm_hdmi_config_avi_infoframe(hdmi);
> > -                     msm_hdmi_audio_update(hdmi);
> > -             }
> > +     }
> > +
> > +     if (hdmi->hdmi_mode) {
> > +             msm_hdmi_config_avi_infoframe(hdmi);
> > +             msm_hdmi_audio_update(hdmi);
> >       }
> >
> >       msm_hdmi_phy_powerup(phy, hdmi->pixclock);
> >
> > --
> > 2.39.2
> >



-- 
With best wishes
Dmitry