| Message ID | 20250319032435.1119469-6-shiyongbang@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add HPD, getting EDID, colorbar features in DP function | expand |
On Wed, 19 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote: > From: Baihan Li <libaihan@huawei.com> > > Add registering drm_aux and use it to get connector edid with drm > functions. Add ddc channel in connector initialization to put drm_aux > in drm_connector. > > Signed-off-by: Baihan Li <libaihan@huawei.com> > Signed-off-by: Yongbang Shi <shiyongbang@huawei.com> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > ChangeLog: > v6 -> v7: > - add if statement about drm aux in hibmc_dp_connector_get_modes(), suggested by Jani Nikula I don't understand this, and I did not suggest such a thing. BR, Jani. > v5 -> v6: > - move the detect_ctx() to the patch 7/9. > v2 -> v3: > - Capitalized EDID and AUX, suggested by Dmitry Baryshkov. > v1 -> v2: > - deleting type conversion, suggested by Dmitry Baryshkov. > - deleting hibmc_dp_connector_get_modes() and using drm_connector_helper_get_modes(), suggested by Dmitry Baryshkov. > --- > drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c | 3 +- > .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c | 35 ++++++++++++++++--- > .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 5 +++ > 3 files changed, 37 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c > index ded9e7ce887a..e0bb9b14d9d8 100644 > --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c > +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c > @@ -161,7 +161,8 @@ void hibmc_dp_aux_init(struct hibmc_dp *dp) > HIBMC_DP_MIN_PULSE_NUM); > > dp->aux.transfer = hibmc_dp_aux_xfer; > - dp->aux.is_remote = 0; > + dp->aux.name = kasprintf(GFP_KERNEL, "HIBMC DRM dp aux"); > + dp->aux.drm_dev = dp->drm_dev; > drm_dp_aux_init(&dp->aux); > dp->dp_dev->aux = &dp->aux; > } > diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c > index 603d6b198a54..0256724d8b9b 100644 > --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c > +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c > @@ -15,11 +15,20 @@ > > static int hibmc_dp_connector_get_modes(struct drm_connector *connector) > { > + struct hibmc_dp *dp = to_hibmc_dp(connector); > + const struct drm_edid *drm_edid; > int count; > > - count = drm_add_modes_noedid(connector, connector->dev->mode_config.max_width, > - connector->dev->mode_config.max_height); > - drm_set_preferred_mode(connector, 1024, 768); // temporary implementation > + if (!dp->aux.name) > + return 0; > + > + drm_edid = drm_edid_read_ddc(connector, &dp->aux.ddc); > + > + drm_edid_connector_update(connector, drm_edid); > + > + count = drm_edid_connector_add_modes(connector); > + > + drm_edid_free(drm_edid); > > return count; > } > @@ -28,12 +37,28 @@ static const struct drm_connector_helper_funcs hibmc_dp_conn_helper_funcs = { > .get_modes = hibmc_dp_connector_get_modes, > }; > > +static int hibmc_dp_late_register(struct drm_connector *connector) > +{ > + struct hibmc_dp *dp = to_hibmc_dp(connector); > + > + return drm_dp_aux_register(&dp->aux); > +} > + > +static void hibmc_dp_early_unregister(struct drm_connector *connector) > +{ > + struct hibmc_dp *dp = to_hibmc_dp(connector); > + > + drm_dp_aux_unregister(&dp->aux); > +} > + > static const struct drm_connector_funcs hibmc_dp_conn_funcs = { > .reset = drm_atomic_helper_connector_reset, > .fill_modes = drm_helper_probe_single_connector_modes, > .destroy = drm_connector_cleanup, > .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, > .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > + .late_register = hibmc_dp_late_register, > + .early_unregister = hibmc_dp_early_unregister, > }; > > static inline int hibmc_dp_prepare(struct hibmc_dp *dp, struct drm_display_mode *mode) > @@ -103,8 +128,8 @@ int hibmc_dp_init(struct hibmc_drm_private *priv) > > drm_encoder_helper_add(encoder, &hibmc_dp_encoder_helper_funcs); > > - ret = drm_connector_init(dev, connector, &hibmc_dp_conn_funcs, > - DRM_MODE_CONNECTOR_DisplayPort); > + ret = drm_connector_init_with_ddc(dev, connector, &hibmc_dp_conn_funcs, > + DRM_MODE_CONNECTOR_DisplayPort, &dp->aux.ddc); > if (ret) { > drm_err(dev, "init dp connector failed: %d\n", ret); > return ret; > diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h > index d982f1e4b958..3ddd71aada66 100644 > --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h > +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h > @@ -47,6 +47,11 @@ static inline struct hibmc_vdac *to_hibmc_vdac(struct drm_connector *connector) > return container_of(connector, struct hibmc_vdac, connector); > } > > +static inline struct hibmc_dp *to_hibmc_dp(struct drm_connector *connector) > +{ > + return container_of(connector, struct hibmc_dp, connector); > +} > + > static inline struct hibmc_drm_private *to_hibmc_drm_private(struct drm_device *dev) > { > return container_of(dev, struct hibmc_drm_private, dev);
> On Wed, 19 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote: >> From: Baihan Li <libaihan@huawei.com> >> >> Add registering drm_aux and use it to get connector edid with drm >> functions. Add ddc channel in connector initialization to put drm_aux >> in drm_connector. >> >> Signed-off-by: Baihan Li <libaihan@huawei.com> >> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com> >> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> --- >> ChangeLog: >> v6 -> v7: >> - add if statement about drm aux in hibmc_dp_connector_get_modes(), suggested by Jani Nikula > I don't understand this, and I did not suggest such a thing. > > BR, > Jani. > Hi Jani, Is the modification of v8 correct? >> v5 -> v6: >> - move the detect_ctx() to the patch 7/9. >> v2 -> v3: >> - Capitalized EDID and AUX, suggested by Dmitry Baryshkov. >> v1 -> v2: >> - deleting type conversion, suggested by Dmitry Baryshkov. >> - deleting hibmc_dp_connector_get_modes() and using drm_connector_helper_get_modes(), suggested by Dmitry Baryshkov. >> --- >> drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c | 3 +- >> .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c | 35 ++++++++++++++++--- >> .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 5 +++ >> 3 files changed, 37 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c >> index ded9e7ce887a..e0bb9b14d9d8 100644 >> --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c >> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c >> @@ -161,7 +161,8 @@ void hibmc_dp_aux_init(struct hibmc_dp *dp) >> HIBMC_DP_MIN_PULSE_NUM); >> >> dp->aux.transfer = hibmc_dp_aux_xfer; >> - dp->aux.is_remote = 0; >> + dp->aux.name = kasprintf(GFP_KERNEL, "HIBMC DRM dp aux"); >> + dp->aux.drm_dev = dp->drm_dev; >> drm_dp_aux_init(&dp->aux); >> dp->dp_dev->aux = &dp->aux; >> } >> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c >> index 603d6b198a54..0256724d8b9b 100644 >> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c >> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c >> @@ -15,11 +15,20 @@ >> >> static int hibmc_dp_connector_get_modes(struct drm_connector *connector) >> { >> + struct hibmc_dp *dp = to_hibmc_dp(connector); >> + const struct drm_edid *drm_edid; >> int count; >> >> - count = drm_add_modes_noedid(connector, connector->dev->mode_config.max_width, >> - connector->dev->mode_config.max_height); >> - drm_set_preferred_mode(connector, 1024, 768); // temporary implementation >> + if (!dp->aux.name) >> + return 0; >> + >> + drm_edid = drm_edid_read_ddc(connector, &dp->aux.ddc); >> + >> + drm_edid_connector_update(connector, drm_edid); >> + >> + count = drm_edid_connector_add_modes(connector); >> + >> + drm_edid_free(drm_edid); >> >> return count; >> } >> @@ -28,12 +37,28 @@ static const struct drm_connector_helper_funcs hibmc_dp_conn_helper_funcs = { >> .get_modes = hibmc_dp_connector_get_modes, >> }; >> >> +static int hibmc_dp_late_register(struct drm_connector *connector) >> +{ >> + struct hibmc_dp *dp = to_hibmc_dp(connector); >> + >> + return drm_dp_aux_register(&dp->aux); >> +} >> + >> +static void hibmc_dp_early_unregister(struct drm_connector *connector) >> +{ >> + struct hibmc_dp *dp = to_hibmc_dp(connector); >> + >> + drm_dp_aux_unregister(&dp->aux); >> +} >> + >> static const struct drm_connector_funcs hibmc_dp_conn_funcs = { >> .reset = drm_atomic_helper_connector_reset, >> .fill_modes = drm_helper_probe_single_connector_modes, >> .destroy = drm_connector_cleanup, >> .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, >> .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, >> + .late_register = hibmc_dp_late_register, >> + .early_unregister = hibmc_dp_early_unregister, >> }; >> >> static inline int hibmc_dp_prepare(struct hibmc_dp *dp, struct drm_display_mode *mode) >> @@ -103,8 +128,8 @@ int hibmc_dp_init(struct hibmc_drm_private *priv) >> >> drm_encoder_helper_add(encoder, &hibmc_dp_encoder_helper_funcs); >> >> - ret = drm_connector_init(dev, connector, &hibmc_dp_conn_funcs, >> - DRM_MODE_CONNECTOR_DisplayPort); >> + ret = drm_connector_init_with_ddc(dev, connector, &hibmc_dp_conn_funcs, >> + DRM_MODE_CONNECTOR_DisplayPort, &dp->aux.ddc); >> if (ret) { >> drm_err(dev, "init dp connector failed: %d\n", ret); >> return ret; >> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h >> index d982f1e4b958..3ddd71aada66 100644 >> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h >> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h >> @@ -47,6 +47,11 @@ static inline struct hibmc_vdac *to_hibmc_vdac(struct drm_connector *connector) >> return container_of(connector, struct hibmc_vdac, connector); >> } >> >> +static inline struct hibmc_dp *to_hibmc_dp(struct drm_connector *connector) >> +{ >> + return container_of(connector, struct hibmc_dp, connector); >> +} >> + >> static inline struct hibmc_drm_private *to_hibmc_drm_private(struct drm_device *dev) >> { >> return container_of(dev, struct hibmc_drm_private, dev);
On Mon, 24 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote: >> On Wed, 19 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote: >>> From: Baihan Li <libaihan@huawei.com> >>> >>> Add registering drm_aux and use it to get connector edid with drm >>> functions. Add ddc channel in connector initialization to put drm_aux >>> in drm_connector. >>> >>> Signed-off-by: Baihan Li <libaihan@huawei.com> >>> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com> >>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>> --- >>> ChangeLog: >>> v6 -> v7: >>> - add if statement about drm aux in hibmc_dp_connector_get_modes(), suggested by Jani Nikula >> I don't understand this, and I did not suggest such a thing. >> >> BR, >> Jani. >> > Hi Jani, > > Is the modification of v8 correct? I never received that for whatever reason. > > >>> v5 -> v6: >>> - move the detect_ctx() to the patch 7/9. >>> v2 -> v3: >>> - Capitalized EDID and AUX, suggested by Dmitry Baryshkov. >>> v1 -> v2: >>> - deleting type conversion, suggested by Dmitry Baryshkov. >>> - deleting hibmc_dp_connector_get_modes() and using drm_connector_helper_get_modes(), suggested by Dmitry Baryshkov. >>> --- >>> drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c | 3 +- >>> .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c | 35 ++++++++++++++++--- >>> .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 5 +++ >>> 3 files changed, 37 insertions(+), 6 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c >>> index ded9e7ce887a..e0bb9b14d9d8 100644 >>> --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c >>> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c >>> @@ -161,7 +161,8 @@ void hibmc_dp_aux_init(struct hibmc_dp *dp) >>> HIBMC_DP_MIN_PULSE_NUM); >>> >>> dp->aux.transfer = hibmc_dp_aux_xfer; >>> - dp->aux.is_remote = 0; >>> + dp->aux.name = kasprintf(GFP_KERNEL, "HIBMC DRM dp aux"); >>> + dp->aux.drm_dev = dp->drm_dev; >>> drm_dp_aux_init(&dp->aux); >>> dp->dp_dev->aux = &dp->aux; >>> } >>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c >>> index 603d6b198a54..0256724d8b9b 100644 >>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c >>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c >>> @@ -15,11 +15,20 @@ >>> >>> static int hibmc_dp_connector_get_modes(struct drm_connector *connector) >>> { >>> + struct hibmc_dp *dp = to_hibmc_dp(connector); >>> + const struct drm_edid *drm_edid; >>> int count; >>> >>> - count = drm_add_modes_noedid(connector, connector->dev->mode_config.max_width, >>> - connector->dev->mode_config.max_height); >>> - drm_set_preferred_mode(connector, 1024, 768); // temporary implementation >>> + if (!dp->aux.name) >>> + return 0; >>> + >>> + drm_edid = drm_edid_read_ddc(connector, &dp->aux.ddc); >>> + >>> + drm_edid_connector_update(connector, drm_edid); >>> + >>> + count = drm_edid_connector_add_modes(connector); >>> + >>> + drm_edid_free(drm_edid); >>> >>> return count; >>> } >>> @@ -28,12 +37,28 @@ static const struct drm_connector_helper_funcs hibmc_dp_conn_helper_funcs = { >>> .get_modes = hibmc_dp_connector_get_modes, >>> }; >>> >>> +static int hibmc_dp_late_register(struct drm_connector *connector) >>> +{ >>> + struct hibmc_dp *dp = to_hibmc_dp(connector); >>> + >>> + return drm_dp_aux_register(&dp->aux); >>> +} >>> + >>> +static void hibmc_dp_early_unregister(struct drm_connector *connector) >>> +{ >>> + struct hibmc_dp *dp = to_hibmc_dp(connector); >>> + >>> + drm_dp_aux_unregister(&dp->aux); >>> +} >>> + >>> static const struct drm_connector_funcs hibmc_dp_conn_funcs = { >>> .reset = drm_atomic_helper_connector_reset, >>> .fill_modes = drm_helper_probe_single_connector_modes, >>> .destroy = drm_connector_cleanup, >>> .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, >>> .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, >>> + .late_register = hibmc_dp_late_register, >>> + .early_unregister = hibmc_dp_early_unregister, >>> }; >>> >>> static inline int hibmc_dp_prepare(struct hibmc_dp *dp, struct drm_display_mode *mode) >>> @@ -103,8 +128,8 @@ int hibmc_dp_init(struct hibmc_drm_private *priv) >>> >>> drm_encoder_helper_add(encoder, &hibmc_dp_encoder_helper_funcs); >>> >>> - ret = drm_connector_init(dev, connector, &hibmc_dp_conn_funcs, >>> - DRM_MODE_CONNECTOR_DisplayPort); >>> + ret = drm_connector_init_with_ddc(dev, connector, &hibmc_dp_conn_funcs, >>> + DRM_MODE_CONNECTOR_DisplayPort, &dp->aux.ddc); >>> if (ret) { >>> drm_err(dev, "init dp connector failed: %d\n", ret); >>> return ret; >>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h >>> index d982f1e4b958..3ddd71aada66 100644 >>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h >>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h >>> @@ -47,6 +47,11 @@ static inline struct hibmc_vdac *to_hibmc_vdac(struct drm_connector *connector) >>> return container_of(connector, struct hibmc_vdac, connector); >>> } >>> >>> +static inline struct hibmc_dp *to_hibmc_dp(struct drm_connector *connector) >>> +{ >>> + return container_of(connector, struct hibmc_dp, connector); >>> +} >>> + >>> static inline struct hibmc_drm_private *to_hibmc_drm_private(struct drm_device *dev) >>> { >>> return container_of(dev, struct hibmc_drm_private, dev);
> On Mon, 24 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote: >>> On Wed, 19 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote: >>>> From: Baihan Li <libaihan@huawei.com> >>>> >>>> Add registering drm_aux and use it to get connector edid with drm >>>> functions. Add ddc channel in connector initialization to put drm_aux >>>> in drm_connector. >>>> >>>> Signed-off-by: Baihan Li <libaihan@huawei.com> >>>> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com> >>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>>> --- >>>> ChangeLog: >>>> v6 -> v7: >>>> - add if statement about drm aux in hibmc_dp_connector_get_modes(), suggested by Jani Nikula >>> I don't understand this, and I did not suggest such a thing. >>> >>> BR, >>> Jani. >>> >> Hi Jani, >> >> Is the modification of v8 correct? > I never received that for whatever reason. Here's the link: https://lore.kernel.org/all/20250320101455.2538835-1-shiyongbang@huawei.com/ >> >>>> v5 -> v6: >>>> - move the detect_ctx() to the patch 7/9. >>>> v2 -> v3: >>>> - Capitalized EDID and AUX, suggested by Dmitry Baryshkov. >>>> v1 -> v2: >>>> - deleting type conversion, suggested by Dmitry Baryshkov. >>>> - deleting hibmc_dp_connector_get_modes() and using drm_connector_helper_get_modes(), suggested by Dmitry Baryshkov. >>>> --- >>>> drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c | 3 +- >>>> .../gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c | 35 ++++++++++++++++--- >>>> .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 5 +++ >>>> 3 files changed, 37 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c >>>> index ded9e7ce887a..e0bb9b14d9d8 100644 >>>> --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c >>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c >>>> @@ -161,7 +161,8 @@ void hibmc_dp_aux_init(struct hibmc_dp *dp) >>>> HIBMC_DP_MIN_PULSE_NUM); >>>> >>>> dp->aux.transfer = hibmc_dp_aux_xfer; >>>> - dp->aux.is_remote = 0; >>>> + dp->aux.name = kasprintf(GFP_KERNEL, "HIBMC DRM dp aux"); >>>> + dp->aux.drm_dev = dp->drm_dev; >>>> drm_dp_aux_init(&dp->aux); >>>> dp->dp_dev->aux = &dp->aux; >>>> } >>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c >>>> index 603d6b198a54..0256724d8b9b 100644 >>>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c >>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c >>>> @@ -15,11 +15,20 @@ >>>> >>>> static int hibmc_dp_connector_get_modes(struct drm_connector *connector) >>>> { >>>> + struct hibmc_dp *dp = to_hibmc_dp(connector); >>>> + const struct drm_edid *drm_edid; >>>> int count; >>>> >>>> - count = drm_add_modes_noedid(connector, connector->dev->mode_config.max_width, >>>> - connector->dev->mode_config.max_height); >>>> - drm_set_preferred_mode(connector, 1024, 768); // temporary implementation >>>> + if (!dp->aux.name) >>>> + return 0; >>>> + >>>> + drm_edid = drm_edid_read_ddc(connector, &dp->aux.ddc); >>>> + >>>> + drm_edid_connector_update(connector, drm_edid); >>>> + >>>> + count = drm_edid_connector_add_modes(connector); >>>> + >>>> + drm_edid_free(drm_edid); >>>> >>>> return count; >>>> } >>>> @@ -28,12 +37,28 @@ static const struct drm_connector_helper_funcs hibmc_dp_conn_helper_funcs = { >>>> .get_modes = hibmc_dp_connector_get_modes, >>>> }; >>>> >>>> +static int hibmc_dp_late_register(struct drm_connector *connector) >>>> +{ >>>> + struct hibmc_dp *dp = to_hibmc_dp(connector); >>>> + >>>> + return drm_dp_aux_register(&dp->aux); >>>> +} >>>> + >>>> +static void hibmc_dp_early_unregister(struct drm_connector *connector) >>>> +{ >>>> + struct hibmc_dp *dp = to_hibmc_dp(connector); >>>> + >>>> + drm_dp_aux_unregister(&dp->aux); >>>> +} >>>> + >>>> static const struct drm_connector_funcs hibmc_dp_conn_funcs = { >>>> .reset = drm_atomic_helper_connector_reset, >>>> .fill_modes = drm_helper_probe_single_connector_modes, >>>> .destroy = drm_connector_cleanup, >>>> .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, >>>> .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, >>>> + .late_register = hibmc_dp_late_register, >>>> + .early_unregister = hibmc_dp_early_unregister, >>>> }; >>>> >>>> static inline int hibmc_dp_prepare(struct hibmc_dp *dp, struct drm_display_mode *mode) >>>> @@ -103,8 +128,8 @@ int hibmc_dp_init(struct hibmc_drm_private *priv) >>>> >>>> drm_encoder_helper_add(encoder, &hibmc_dp_encoder_helper_funcs); >>>> >>>> - ret = drm_connector_init(dev, connector, &hibmc_dp_conn_funcs, >>>> - DRM_MODE_CONNECTOR_DisplayPort); >>>> + ret = drm_connector_init_with_ddc(dev, connector, &hibmc_dp_conn_funcs, >>>> + DRM_MODE_CONNECTOR_DisplayPort, &dp->aux.ddc); >>>> if (ret) { >>>> drm_err(dev, "init dp connector failed: %d\n", ret); >>>> return ret; >>>> diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h >>>> index d982f1e4b958..3ddd71aada66 100644 >>>> --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h >>>> +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h >>>> @@ -47,6 +47,11 @@ static inline struct hibmc_vdac *to_hibmc_vdac(struct drm_connector *connector) >>>> return container_of(connector, struct hibmc_vdac, connector); >>>> } >>>> >>>> +static inline struct hibmc_dp *to_hibmc_dp(struct drm_connector *connector) >>>> +{ >>>> + return container_of(connector, struct hibmc_dp, connector); >>>> +} >>>> + >>>> static inline struct hibmc_drm_private *to_hibmc_drm_private(struct drm_device *dev) >>>> { >>>> return container_of(dev, struct hibmc_drm_private, dev);
On Tue, 25 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote: >> On Mon, 24 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote: >>>> On Wed, 19 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote: >>>>> From: Baihan Li <libaihan@huawei.com> >>>>> >>>>> Add registering drm_aux and use it to get connector edid with drm >>>>> functions. Add ddc channel in connector initialization to put drm_aux >>>>> in drm_connector. >>>>> >>>>> Signed-off-by: Baihan Li <libaihan@huawei.com> >>>>> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com> >>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>>>> --- >>>>> ChangeLog: >>>>> v6 -> v7: >>>>> - add if statement about drm aux in hibmc_dp_connector_get_modes(), suggested by Jani Nikula >>>> I don't understand this, and I did not suggest such a thing. >>>> >>>> BR, >>>> Jani. >>>> >>> Hi Jani, >>> >>> Is the modification of v8 correct? >> I never received that for whatever reason. > > Here's the link: https://lore.kernel.org/all/20250320101455.2538835-1-shiyongbang@huawei.com/ Thanks. The EDID handling looks fine. AFAICT you leak dp->aux.name though. BR, Jani.
在 2025/3/26 17:32, Jani Nikula 写道: > On Tue, 25 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote: >>> On Mon, 24 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote: >>>>> On Wed, 19 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote: >>>>>> From: Baihan Li <libaihan@huawei.com> >>>>>> >>>>>> Add registering drm_aux and use it to get connector edid with drm >>>>>> functions. Add ddc channel in connector initialization to put drm_aux >>>>>> in drm_connector. >>>>>> >>>>>> Signed-off-by: Baihan Li <libaihan@huawei.com> >>>>>> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com> >>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>>>>> --- >>>>>> ChangeLog: >>>>>> v6 -> v7: >>>>>> - add if statement about drm aux in hibmc_dp_connector_get_modes(), suggested by Jani Nikula >>>>> I don't understand this, and I did not suggest such a thing. >>>>> >>>>> BR, >>>>> Jani. >>>>> >>>> Hi Jani, >>>> >>>> Is the modification of v8 correct? >>> I never received that for whatever reason. >> Here's the link: https://lore.kernel.org/all/20250320101455.2538835-1-shiyongbang@huawei.com/ > Thanks. > > The EDID handling looks fine. > > AFAICT you leak dp->aux.name though. > > > BR, > Jani. Thanks for for reminding me, actually the dp->aux.name was written because I misunderstood what you meant in V7, and I deleted it in V8. Thanks, Baihan. > >
On Thu, 27 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote: > 在 2025/3/26 17:32, Jani Nikula 写道: >> On Tue, 25 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote: >>>> On Mon, 24 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote: >>>>>> On Wed, 19 Mar 2025, Yongbang Shi <shiyongbang@huawei.com> wrote: >>>>>>> From: Baihan Li <libaihan@huawei.com> >>>>>>> >>>>>>> Add registering drm_aux and use it to get connector edid with drm >>>>>>> functions. Add ddc channel in connector initialization to put drm_aux >>>>>>> in drm_connector. >>>>>>> >>>>>>> Signed-off-by: Baihan Li <libaihan@huawei.com> >>>>>>> Signed-off-by: Yongbang Shi <shiyongbang@huawei.com> >>>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >>>>>>> --- >>>>>>> ChangeLog: >>>>>>> v6 -> v7: >>>>>>> - add if statement about drm aux in hibmc_dp_connector_get_modes(), suggested by Jani Nikula >>>>>> I don't understand this, and I did not suggest such a thing. >>>>>> >>>>>> BR, >>>>>> Jani. >>>>>> >>>>> Hi Jani, >>>>> >>>>> Is the modification of v8 correct? >>>> I never received that for whatever reason. >>> Here's the link: https://lore.kernel.org/all/20250320101455.2538835-1-shiyongbang@huawei.com/ >> Thanks. >> >> The EDID handling looks fine. >> >> AFAICT you leak dp->aux.name though. >> >> >> BR, >> Jani. > > Thanks for for reminding me, actually the dp->aux.name was written because I misunderstood what you meant in V7, > and I deleted it in V8. This is in the link you posted: + dp->aux.name = kasprintf(GFP_KERNEL, "HIBMC DRM dp aux"); > > Thanks, > Baihan. > >> >>
diff --git a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c index ded9e7ce887a..e0bb9b14d9d8 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c +++ b/drivers/gpu/drm/hisilicon/hibmc/dp/dp_aux.c @@ -161,7 +161,8 @@ void hibmc_dp_aux_init(struct hibmc_dp *dp) HIBMC_DP_MIN_PULSE_NUM); dp->aux.transfer = hibmc_dp_aux_xfer; - dp->aux.is_remote = 0; + dp->aux.name = kasprintf(GFP_KERNEL, "HIBMC DRM dp aux"); + dp->aux.drm_dev = dp->drm_dev; drm_dp_aux_init(&dp->aux); dp->dp_dev->aux = &dp->aux; } diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c index 603d6b198a54..0256724d8b9b 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_dp.c @@ -15,11 +15,20 @@ static int hibmc_dp_connector_get_modes(struct drm_connector *connector) { + struct hibmc_dp *dp = to_hibmc_dp(connector); + const struct drm_edid *drm_edid; int count; - count = drm_add_modes_noedid(connector, connector->dev->mode_config.max_width, - connector->dev->mode_config.max_height); - drm_set_preferred_mode(connector, 1024, 768); // temporary implementation + if (!dp->aux.name) + return 0; + + drm_edid = drm_edid_read_ddc(connector, &dp->aux.ddc); + + drm_edid_connector_update(connector, drm_edid); + + count = drm_edid_connector_add_modes(connector); + + drm_edid_free(drm_edid); return count; } @@ -28,12 +37,28 @@ static const struct drm_connector_helper_funcs hibmc_dp_conn_helper_funcs = { .get_modes = hibmc_dp_connector_get_modes, }; +static int hibmc_dp_late_register(struct drm_connector *connector) +{ + struct hibmc_dp *dp = to_hibmc_dp(connector); + + return drm_dp_aux_register(&dp->aux); +} + +static void hibmc_dp_early_unregister(struct drm_connector *connector) +{ + struct hibmc_dp *dp = to_hibmc_dp(connector); + + drm_dp_aux_unregister(&dp->aux); +} + static const struct drm_connector_funcs hibmc_dp_conn_funcs = { .reset = drm_atomic_helper_connector_reset, .fill_modes = drm_helper_probe_single_connector_modes, .destroy = drm_connector_cleanup, .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, + .late_register = hibmc_dp_late_register, + .early_unregister = hibmc_dp_early_unregister, }; static inline int hibmc_dp_prepare(struct hibmc_dp *dp, struct drm_display_mode *mode) @@ -103,8 +128,8 @@ int hibmc_dp_init(struct hibmc_drm_private *priv) drm_encoder_helper_add(encoder, &hibmc_dp_encoder_helper_funcs); - ret = drm_connector_init(dev, connector, &hibmc_dp_conn_funcs, - DRM_MODE_CONNECTOR_DisplayPort); + ret = drm_connector_init_with_ddc(dev, connector, &hibmc_dp_conn_funcs, + DRM_MODE_CONNECTOR_DisplayPort, &dp->aux.ddc); if (ret) { drm_err(dev, "init dp connector failed: %d\n", ret); return ret; diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h index d982f1e4b958..3ddd71aada66 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h @@ -47,6 +47,11 @@ static inline struct hibmc_vdac *to_hibmc_vdac(struct drm_connector *connector) return container_of(connector, struct hibmc_vdac, connector); } +static inline struct hibmc_dp *to_hibmc_dp(struct drm_connector *connector) +{ + return container_of(connector, struct hibmc_dp, connector); +} + static inline struct hibmc_drm_private *to_hibmc_drm_private(struct drm_device *dev) { return container_of(dev, struct hibmc_drm_private, dev);