lkml.org 
[lkml]   [2022]   [Jun]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v1 2/3] drm/msm/dp: decoupling dp->id out of dp controller_id at scxxxx_dp_cfg table
    On Sat, 25 Jun 2022 at 00:17, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote:
    >
    >
    > On 6/24/2022 1:00 PM, Stephen Boyd wrote:
    > > Quoting Kuogee Hsieh (2022-06-24 10:15:11)
    > >> Current the index (dp->id) of DP descriptor table (scxxxx_dp_cfg[]) are tightly
    > >> coupled with DP controller_id. This means DP use controller id 0 must be placed
    > >> at first entry of DP descriptor table (scxxxx_dp_cfg[]). Otherwise the internal
    > >> INTF will mismatch controller_id. This will cause controller kickoff wrong
    > >> interface timing engine and cause dpu_encoder_phys_vid_wait_for_commit_done
    > >> vblank timeout error.
    > >>
    > >> This patch add controller_id field into struct msm_dp_desc to break the tightly
    > >> coupled relationship between index (dp->id) of DP descriptor table with DP
    > >> controller_id.
    > > Please no. This reverts the intention of commit bb3de286d992
    > > ("drm/msm/dp: Support up to 3 DP controllers")
    > >
    > > A new enum is introduced to document the connection between the
    > > instances referenced in the dpu_intf_cfg array and the controllers in
    > > the DP driver and sc7180 is updated.
    > >
    > > It sounds like the intent of that commit failed to make a strong enough
    > > connection. Now it needs to match the INTF number as well? I can't
    > > really figure out what is actually wrong, because this patch undoes that
    > > intentional tight coupling. Is the next patch the important part that
    > > flips the order of the two interfaces?
    >
    > The commit bb3de286d992have two problems,
    >
    > 1) The below sc7280_dp_cfg will not work, if eDP use
    > MSM_DP_CONTROLLER_2 instead of MSM_DP_CONTROLLER_1
    >
    > since it have num_descs =2 but eDP is at index 2 (CONTROLLER_2) which
    > never be reached.
    >
    > static const struct msm_dp_config sc7280_dp_cfg = {
    > .descs = (const struct msm_dp_desc[]) {
    > [MSM_DP_CONTROLLER_2] = { .io_start = 0x0aea0000,
    > .connector_type = DRM_MODE_CONNECTOR_eDP, .wide_bus_en = true },
    > [MSM_DP_CONTROLLER_0] = { .io_start = 0x0ae90000,
    > .connector_type = DRM_MODE_CONNECTOR_DisplayPort, .wide_bus_en = true },
    > },
    > .num_descs = 2,

    Please change num_descs to 3. Or better eliminate it completely and
    iterate up to MSM_DP_CONTROLLER_MAX, checking whether the entry
    contains real values or is just a zero sentinel entry.

    > };
    >
    > 2) DP always has index of 0 (dp->id = 0) and the first one to call
    > msm_dp_modeset_init(). This make DP always place at head of bridge chain.
    >
    > At next patch eDP must be placed at head of bridge chain to fix eDP
    > corruption issue. This is the purpose of this patch. I will revise the
    > commit text.

    This text doesn't make sense to me. The dp->id has nothing to do with
    the bridge chains. Each dp entry is a head of the corresponding bridge
    chain. DP with dp->id = 0 and eDP with dp->id = whatever will be parts
    of different encoder -> bridges -> connector chains.

    --
    With best wishes
    Dmitry

    \
     
     \ /
      Last update: 2022-06-25 01:26    [W:3.696 / U:0.424 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site