On Wed, 28 May 2025 at 04:57, Chaoyi Chen <chaoyi.c...@rock-chips.com> wrote:
>
> Hi Dmitry,
>
> On 2025/5/28 4:25, Dmitry Baryshkov wrote:
> > On Tue, May 27, 2025 at 04:14:47PM +0800, Chaoyi Chen wrote:
> >> From: Chaoyi Chen <chaoyi.c...@rock-chips.com>
> >>
> >> Convert it to drm bridge driver, it will be convenient for us to
> >> migrate the connector part to the display driver later.
> >> Considering that some code depend on the connector, the following
> >> changes have been made:
> >> - Only process edid in &drm_bridge_funcs.edid_read(), so no need to
> >> store additional edid info.
> >> - Now cdn_dp_get_sink_capability() only focused on reading DPCD_REV.
> >> - Update bpc info in cdn_dp_bridge_atomic_enable() instead of
> >> cdn_dp_encoder_mode_set(). Actually, the bpc data will be used in
> >> cdn_dp_bridge_atomic_enable().
> >> - Switch to use DRM_BRIDGE_OP_DP_AUDIO helpers.
> >>
> >> This patch also convert to use devm_drm_bridge_alloc() API.
> >>
> >> Tested with RK3399 EVB IND board.
> >>
> >> Signed-off-by: Chaoyi Chen <chaoyi.c...@rock-chips.com>
> >> ---
> >>
> >> Changes in v3:
> >> - Link to V2:
> >> https://lore.kernel.org/all/20250523011310.120-1-ker...@airkyi.com/
> >> - Switch to use DRM_BRIDGE_OP_DP_AUDIO helpers
> >> - Remove the dependency for connector
> >> - Remove the extra stored edid
> >> - Code cleanup
> >>
> >> Changes in v2:
> >> - Link to V1:
> >> https://lore.kernel.org/all/20250507035148.415-1-ker...@airkyi.com/
> >> - Use drm_atomic_get_new_connector_for_encoder() to get connector
> >> - Convert to use devm_drm_bridge_alloc() API
> >> - Fix typo: cdn_dp_connector_edid_read -> cdn_dp_bridge_edid_read
> >>
> >> drivers/gpu/drm/rockchip/cdn-dp-core.c | 279 ++++++++++---------------
> >> drivers/gpu/drm/rockchip/cdn-dp-core.h | 9 +-
> >> 2 files changed, 110 insertions(+), 178 deletions(-)
> >>
> >
> >> @@ -595,16 +546,41 @@ static bool cdn_dp_check_link_status(struct
> >> cdn_dp_device *dp)
> >> static void cdn_dp_audio_handle_plugged_change(struct cdn_dp_device *dp,
> >> bool plugged)
> >> {
> >> - if (dp->codec_dev)
> >> - dp->plugged_cb(dp->codec_dev, plugged);
> >> + if (dp->sink_has_audio)
> >> + drm_connector_hdmi_audio_plugged_notify(dp->connector,
> >> plugged);
> > I'd say, notify always and let userspace figure it out via the ELD. Then
> > you shouldn't need sink_has_audio. This would match the behaviour of
> > HDMI drivers.
>
> Oh, I find that there are similar usages in qcom msm driver. Is there
> any more progress?
For msm driver it is required as DSP requires HDMI to be plugged for
the audio path to work.
>
>
> >
> >> }
> >>
> > [...]
> >
> >> @@ -705,8 +681,6 @@ static int cdn_dp_encoder_atomic_check(struct
> >> drm_encoder *encoder,
> >>
> >> static const struct drm_encoder_helper_funcs cdn_dp_encoder_helper_funcs
> >> = {
> >> .mode_set = cdn_dp_encoder_mode_set,
> >> - .enable = cdn_dp_encoder_enable,
> >> - .disable = cdn_dp_encoder_disable,
> >> .atomic_check = cdn_dp_encoder_atomic_check,
> > Nit: for the future cleanup, it should probably be possible to get rid
> > of these encoder ops too by moving them to the bridge ops.
>
> Interesting, have these patches been submitted upstream yet?
Everything is already there, see drm_bridge_funcs::mode_set() and
drm_bridge_funcs::atomic_check().
--
With best wishes
Dmitry
