[Freedreno] [PATCH v2 02/13] drm/msm/hdmi: switch to devm_drm_bridge_add()
Abhinav Kumar
quic_abhinavk at quicinc.com
Mon Oct 9 19:50:42 UTC 2023
On 10/9/2023 12:21 PM, Dmitry Baryshkov wrote:
> On 09/10/2023 22:19, Abhinav Kumar wrote:
>>
>>
>> On 10/9/2023 11:10 AM, Dmitry Baryshkov wrote:
>>> Make MSM HDMI driver use devm_drm_bridge_add() instead of plain
>>> drm_bridge_add(). As the driver doesn't require any additional cleanup,
>>> stop adding created bridge to the priv->bridges array.
>>>
>>> Reviewed-by: Rob Clark <robdclark at gmail.com>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>>> ---
>>> drivers/gpu/drm/msm/hdmi/hdmi.c | 22 +++++--------------
>>> drivers/gpu/drm/msm/hdmi/hdmi.h | 5 ++---
>>> drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 30 ++++++++------------------
>>> drivers/gpu/drm/msm/hdmi/hdmi_hpd.c | 3 +--
>>> 4 files changed, 17 insertions(+), 43 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c
>>> b/drivers/gpu/drm/msm/hdmi/hdmi.c
>>> index b6bcb9f675fe..c8ebd75176bb 100644
>>> --- a/drivers/gpu/drm/msm/hdmi/hdmi.c
>>> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
>>> @@ -160,24 +160,16 @@ static int msm_hdmi_init(struct hdmi *hdmi)
>>> int msm_hdmi_modeset_init(struct hdmi *hdmi,
>>> struct drm_device *dev, struct drm_encoder *encoder)
>>> {
>>> - struct msm_drm_private *priv = dev->dev_private;
>>> int ret;
>>> - if (priv->num_bridges == ARRAY_SIZE(priv->bridges)) {
>>> - DRM_DEV_ERROR(dev->dev, "too many bridges\n");
>>> - return -ENOSPC;
>>> - }
>>> -
>>> hdmi->dev = dev;
>>> hdmi->encoder = encoder;
>>> hdmi_audio_infoframe_init(&hdmi->audio.infoframe);
>>> - hdmi->bridge = msm_hdmi_bridge_init(hdmi);
>>> - if (IS_ERR(hdmi->bridge)) {
>>> - ret = PTR_ERR(hdmi->bridge);
>>> + ret = msm_hdmi_bridge_init(hdmi);
>>> + if (ret) {
>>> DRM_DEV_ERROR(dev->dev, "failed to create HDMI bridge:
>>> %d\n", ret);
>>> - hdmi->bridge = NULL;
>>> goto fail;
>>> }
>>> @@ -215,16 +207,9 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
>>> goto fail;
>>> }
>>> - priv->bridges[priv->num_bridges++] = hdmi->bridge;
>>> -
>>> return 0;
>>> fail:
>>> - /* bridge is normally destroyed by drm: */
>>> - if (hdmi->bridge) {
>>> - msm_hdmi_bridge_destroy(hdmi->bridge);
>>> - hdmi->bridge = NULL;
>>> - }
>>> if (hdmi->connector) {
>>> hdmi->connector->funcs->destroy(hdmi->connector);
>>> hdmi->connector = NULL;
>>> @@ -395,6 +380,9 @@ static void msm_hdmi_unbind(struct device *dev,
>>> struct device *master,
>>> if (priv->hdmi->audio_pdev)
>>> platform_device_unregister(priv->hdmi->audio_pdev);
>>> + if (priv->hdmi->bridge)
>>> + msm_hdmi_hpd_disable(priv->hdmi);
>>> +
>>
>> Now is this the only place where hdmi->bridge is used?
>>
>> Why cant we just keep msm_hdmi_hpd_disable(priv->hdmi) here since its
>> anyway protected by if (priv->hdmi) and drop hdmi->bridge completely?
>
> Sure, sounds like a good idea, same followup as for the DSI.
Ok, this is fine then,
Reviewed-by: Abhinav Kumar <quic_abhinavk at quicinc.com>
>
>>
>>> msm_hdmi_destroy(priv->hdmi);
>>> priv->hdmi = NULL;
>>> }
>>> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h
>>> b/drivers/gpu/drm/msm/hdmi/hdmi.h
>>> index e8dbee50637f..ec5786440391 100644
>>> --- a/drivers/gpu/drm/msm/hdmi/hdmi.h
>>> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
>>> @@ -224,14 +224,13 @@ void msm_hdmi_audio_set_sample_rate(struct hdmi
>>> *hdmi, int rate);
>>> * hdmi bridge:
>>> */
>>> -struct drm_bridge *msm_hdmi_bridge_init(struct hdmi *hdmi);
>>> -void msm_hdmi_bridge_destroy(struct drm_bridge *bridge);
>>> +int msm_hdmi_bridge_init(struct hdmi *hdmi);
>>> void msm_hdmi_hpd_irq(struct drm_bridge *bridge);
>>> enum drm_connector_status msm_hdmi_bridge_detect(
>>> struct drm_bridge *bridge);
>>> int msm_hdmi_hpd_enable(struct drm_bridge *bridge);
>>> -void msm_hdmi_hpd_disable(struct hdmi_bridge *hdmi_bridge);
>>> +void msm_hdmi_hpd_disable(struct hdmi *hdmi);
>>> /*
>>> * i2c adapter for ddc:
>>> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
>>> b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
>>> index 9b1391d27ed3..0b7a6a56677e 100644
>>> --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
>>> +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
>>> @@ -11,14 +11,6 @@
>>> #include "msm_kms.h"
>>> #include "hdmi.h"
>>> -void msm_hdmi_bridge_destroy(struct drm_bridge *bridge)
>>> -{
>>> - struct hdmi_bridge *hdmi_bridge = to_hdmi_bridge(bridge);
>>> -
>>> - msm_hdmi_hpd_disable(hdmi_bridge);
>>> - drm_bridge_remove(bridge);
>>> -}
>>> -
>>> static void msm_hdmi_power_on(struct drm_bridge *bridge)
>>> {
>>> struct drm_device *dev = bridge->dev;
>>> @@ -317,7 +309,7 @@ msm_hdmi_hotplug_work(struct work_struct *work)
>>> }
>>> /* initialize bridge */
>>> -struct drm_bridge *msm_hdmi_bridge_init(struct hdmi *hdmi)
>>> +int msm_hdmi_bridge_init(struct hdmi *hdmi)
>>> {
>>> struct drm_bridge *bridge = NULL;
>>> struct hdmi_bridge *hdmi_bridge;
>>> @@ -325,10 +317,8 @@ struct drm_bridge *msm_hdmi_bridge_init(struct
>>> hdmi *hdmi)
>>> hdmi_bridge = devm_kzalloc(hdmi->dev->dev,
>>> sizeof(*hdmi_bridge), GFP_KERNEL);
>>> - if (!hdmi_bridge) {
>>> - ret = -ENOMEM;
>>> - goto fail;
>>> - }
>>> + if (!hdmi_bridge)
>>> + return -ENOMEM;
>>> hdmi_bridge->hdmi = hdmi;
>>> INIT_WORK(&hdmi_bridge->hpd_work, msm_hdmi_hotplug_work);
>>> @@ -341,17 +331,15 @@ struct drm_bridge *msm_hdmi_bridge_init(struct
>>> hdmi *hdmi)
>>> DRM_BRIDGE_OP_DETECT |
>>> DRM_BRIDGE_OP_EDID;
>>> - drm_bridge_add(bridge);
>>> + ret = devm_drm_bridge_add(&hdmi->pdev->dev, bridge);
>>> + if (ret)
>>> + return ret;
>>> ret = drm_bridge_attach(hdmi->encoder, bridge, NULL,
>>> DRM_BRIDGE_ATTACH_NO_CONNECTOR);
>>> if (ret)
>>> - goto fail;
>>> + return ret;
>>> - return bridge;
>>> + hdmi->bridge = bridge;
>>> -fail:
>>> - if (bridge)
>>> - msm_hdmi_bridge_destroy(bridge);
>>> -
>>> - return ERR_PTR(ret);
>>> + return 0;
>>> }
>>> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
>>> b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
>>> index bfa827b47989..9ce0ffa35417 100644
>>> --- a/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
>>> +++ b/drivers/gpu/drm/msm/hdmi/hdmi_hpd.c
>>> @@ -147,9 +147,8 @@ int msm_hdmi_hpd_enable(struct drm_bridge *bridge)
>>> return ret;
>>> }
>>> -void msm_hdmi_hpd_disable(struct hdmi_bridge *hdmi_bridge)
>>> +void msm_hdmi_hpd_disable(struct hdmi *hdmi)
>>> {
>>> - struct hdmi *hdmi = hdmi_bridge->hdmi;
>>> const struct hdmi_platform_config *config = hdmi->config;
>>> struct device *dev = &hdmi->pdev->dev;
>>> int ret;
>
More information about the Freedreno
mailing list