[PATCH v3] drm/rockchip: cdn-dp: Convert to drm bridge

[Dmitry Baryshkov]

On Wed, 28 May 2025 at 04:57, Chaoyi Chen <chaoyi.chen at 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.chen at 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.chen at rock-chips.com>
> >> ---
> >>
> >> Changes in v3:
> >> - Link to V2: https://lore.kernel.org/all/20250523011310.120-1-kernel@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-kernel@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