With the DBANC framework, the connector is no longer initialized in 
bridge_attach() when the display controller sets the 
DRM_BRIDGE_ATTACH_NO_CONNECTOR flag. 
This causes a null pointer dereference in cdns_mhdp_modeset_retry_fn() 
when trying to access &conn->dev->mode_config.mutex. 
Observed on a board where EDID read failed. 
(log: https://gist.github.com/Jayesh2000/233f87f9becdf1e66f1da6fd53f77429)

Patch 1 adds a connector_ptr which takes care of both 
DBANC and !DBANC case by setting the pointer in appropriate hooks
and checking for pointer validity before accessing the connector.
Patch 2 adds mode validation hook to bridge fucntions.
Patch 3 fixes HDCP to work with both DBANC and !DBANC case by
moving HDCP state handling into the bridge atomic check in line with 
the DBANC model.
Patches 4,5 do necessary cleanup and alignment for using
connector pointer.

The rationale behind the sequence of commits is we can cleanly 
switch to drm_connector pointer after removal of connector helper 
code blocks, which are anyways not touch after DBANC has been 
enabled in driver.

The last patch make smaller adjustments: lowering the log level for
noisy DPCD transfer errors.

v8 patch link:
<https://lore.kernel.org/all/20251014094527.3916421-1-h-shenoy@ti.com/>

Changelog v8-v9:
-Move the patch 6 in v8 related to HDCP to patch 3 and add fixes tag.
-Update to connector_ptr in HDCP code in patch 1.
-Rebased on next-20251114.

v7 patch link:
<https://lore.kernel.org/all/20250929083936.1575685-1-h-shenoy@ti.com/>

Changelog v7-v8:
-Move patches with firxes tag to top of series with appropriate changes
to them.
-Add R/B tag to patch 
https://lore.kernel.org/all/ae3snoap64r252sbqhsshsadxfmlqdfn6b4o5fgfcmxppglkqf@2lsstfsghzwb/

v6 patch link:
<https://lore.kernel.org/all/20250909090824.1655537-1-h-shenoy@ti.com/>

Changelog v6-v7:
-Update cover letter to explain the series.
-Add R/B tag in PATCH 1 and drop fixes tag as suggested.
-Drop fixes tag in PATCH 2.
-Update the commit messages for clear understanding of changes done in patches.

v5 patch link:
<https://lore.kernel.org/all/20250811075904.1613519-1-h-shenoy@ti.com/>

Changelog v5 -> v6:
-Update cover letter to clarify the series in better way.
-Add Reviewed-by tag to relevant patches.
 
v4 patch link: 
<https://lore.kernel.org/all/20250624054448.192801-1-j-choudhary@ti.com>

Changelog v4->v5:
- Handle HDCP state in bridge atomic check instead of connector 
atomic check
 
v3 patch link:
<https://lore.kernel.org/all/20250529142517.188786-1-j-choudhary@ti.com/>

Changelog v3->v4:
- Fix kernel test robot build warning:
  <https://lore.kernel.org/all/202505300201.2s6r12yc-lkp@intel.com/>

v2 patch link:
<https://lore.kernel.org/all/20250521073237.366463-1-j-choudhary@ti.com/>

Changelog v2->v3:
- Add mode_valid in drm_bridge_funcs to a separate patch
- Remove "if (mhdp->connector.dev)" conditions that were missed in v2
- Split out the move of drm_atomic_get_new_connector_for_encoder()
  to a separate patch
- Drop "R-by" considering the changes in v2[1/3]
- Add Fixes tag to first 4 patches:
  commit c932ced6b585 ("drm/tidss: Update encoder/bridge chain connect model")
  This added DBANC flag in tidss while attaching bridge to the encoder
- Drop RFC prefix

v1 patch link:
<https://lore.kernel.org/all/20250116111636.157641-1-j-choudhary@ti.com/>

Changelog v1->v2:
- Remove !DRM_BRIDGE_ATTACH_NO_CONNECTOR entirely
- Add mode_valid in drm_bridge_funcs[0]
- Fix NULL POINTER differently since we cannot access atomic_state
- Reduce log level in cdns_mhdp_transfer call

[0]: https://lore.kernel.org/all/20240530091757.433106-1-j-choudhary@ti.com/

Harikrishna Shenoy (1):
  drm/bridge: cadence: cdns-mhdp8546-core: Handle HDCP state in bridge
    atomic check

Jayesh Choudhary (5):
  drm/bridge: cadence: cdns-mhdp8546-core: Set the mhdp connector
    earlier in atomic_enable()
  drm/bridge: cadence: cdns-mhdp8546-core: Add mode_valid hook to
    drm_bridge_funcs
  drm/bridge: cadence: cdns-mhdp8546-core: Remove legacy support for
    connector initialisation in bridge
  drm/bridge: cadence: cdns-mhdp8546*: Change drm_connector from
    structure to pointer
  drm/bridge: cadence: cdns-mhdp8546-core: Reduce log level for DPCD
    read/write

 .../drm/bridge/cadence/cdns-mhdp8546-core.c   | 258 +++++-------------
 .../drm/bridge/cadence/cdns-mhdp8546-core.h   |   2 +-
 .../drm/bridge/cadence/cdns-mhdp8546-hdcp.c   |   8 +-
 3 files changed, 72 insertions(+), 196 deletions(-)

-- 
2.34.1

Hi,

On Tue, Nov 18, 2025 at 05:22:49PM +0530, Harikrishna Shenoy wrote:
> With the DBANC framework, the connector is no longer initialized in 
> bridge_attach() when the display controller sets the 
> DRM_BRIDGE_ATTACH_NO_CONNECTOR flag. 
> This causes a null pointer dereference in cdns_mhdp_modeset_retry_fn() 
> when trying to access &conn->dev->mode_config.mutex. 
> Observed on a board where EDID read failed. 
> (log: https://gist.github.com/Jayesh2000/233f87f9becdf1e66f1da6fd53f77429)
> 
> Patch 1 adds a connector_ptr which takes care of both 
> DBANC and !DBANC case by setting the pointer in appropriate hooks
> and checking for pointer validity before accessing the connector.
> Patch 2 adds mode validation hook to bridge fucntions.
> Patch 3 fixes HDCP to work with both DBANC and !DBANC case by
> moving HDCP state handling into the bridge atomic check in line with 
> the DBANC model.
> Patches 4,5 do necessary cleanup and alignment for using
> connector pointer.

It's mentioned several times in your series, and it might be obvious to
you, but documenting what is the "DBANC framework" is would be helpful.
I have no idea what it's about, and it appears that Google doesn't know
either.

Maxime

Hi,

On 18/11/2025 14:40, Maxime Ripard wrote:
> Hi,
> 
> On Tue, Nov 18, 2025 at 05:22:49PM +0530, Harikrishna Shenoy wrote:
>> With the DBANC framework, the connector is no longer initialized in 
>> bridge_attach() when the display controller sets the 
>> DRM_BRIDGE_ATTACH_NO_CONNECTOR flag. 
>> This causes a null pointer dereference in cdns_mhdp_modeset_retry_fn() 
>> when trying to access &conn->dev->mode_config.mutex. 
>> Observed on a board where EDID read failed. 
>> (log: https://gist.github.com/Jayesh2000/233f87f9becdf1e66f1da6fd53f77429)
>>
>> Patch 1 adds a connector_ptr which takes care of both 
>> DBANC and !DBANC case by setting the pointer in appropriate hooks
>> and checking for pointer validity before accessing the connector.
>> Patch 2 adds mode validation hook to bridge fucntions.
>> Patch 3 fixes HDCP to work with both DBANC and !DBANC case by
>> moving HDCP state handling into the bridge atomic check in line with 
>> the DBANC model.
>> Patches 4,5 do necessary cleanup and alignment for using
>> connector pointer.
> 
> It's mentioned several times in your series, and it might be obvious to
> you, but documenting what is the "DBANC framework" is would be helpful.
> I have no idea what it's about, and it appears that Google doesn't know
> either.
Yes, I was a bit baffled initially. DRM_BRIDGE_ATTACH_NO_CONNECTOR.

I think it makes sense to only use "DBANC" if it's first introduced in
that patch. So don't have a patch that just uses "DBANC", even if the
previous patch did explain what it means. And if there's just one or two
"DBANC"s, just spell it out "DRM_BRIDGE_ATTACH_NO_CONNECTOR".

 Tomi

On Tue, Nov 18, 2025 at 04:52:49PM +0200, Tomi Valkeinen wrote:
> Hi,
> 
> On 18/11/2025 14:40, Maxime Ripard wrote:
> > Hi,
> > 
> > On Tue, Nov 18, 2025 at 05:22:49PM +0530, Harikrishna Shenoy wrote:
> >> With the DBANC framework, the connector is no longer initialized in 
> >> bridge_attach() when the display controller sets the 
> >> DRM_BRIDGE_ATTACH_NO_CONNECTOR flag. 
> >> This causes a null pointer dereference in cdns_mhdp_modeset_retry_fn() 
> >> when trying to access &conn->dev->mode_config.mutex. 
> >> Observed on a board where EDID read failed. 
> >> (log: https://gist.github.com/Jayesh2000/233f87f9becdf1e66f1da6fd53f77429)
> >>
> >> Patch 1 adds a connector_ptr which takes care of both 
> >> DBANC and !DBANC case by setting the pointer in appropriate hooks
> >> and checking for pointer validity before accessing the connector.
> >> Patch 2 adds mode validation hook to bridge fucntions.
> >> Patch 3 fixes HDCP to work with both DBANC and !DBANC case by
> >> moving HDCP state handling into the bridge atomic check in line with 
> >> the DBANC model.
> >> Patches 4,5 do necessary cleanup and alignment for using
> >> connector pointer.
> > 
> > It's mentioned several times in your series, and it might be obvious to
> > you, but documenting what is the "DBANC framework" is would be helpful.
> > I have no idea what it's about, and it appears that Google doesn't know
> > either.
> Yes, I was a bit baffled initially. DRM_BRIDGE_ATTACH_NO_CONNECTOR.

Oooooh, thanks

> I think it makes sense to only use "DBANC" if it's first introduced in
> that patch. So don't have a patch that just uses "DBANC", even if the
> previous patch did explain what it means. And if there's just one or two
> "DBANC"s, just spell it out "DRM_BRIDGE_ATTACH_NO_CONNECTOR".

Yeah, I'd go even further. Acronyms are fun but something being obvious
is better still. Use the proper flag name every time.

Maxime

On 18/11/25 21:50, Maxime Ripard wrote:
> On Tue, Nov 18, 2025 at 04:52:49PM +0200, Tomi Valkeinen wrote:
>> Hi,
>>
>> On 18/11/2025 14:40, Maxime Ripard wrote:
>>> Hi,
>>>
>>> On Tue, Nov 18, 2025 at 05:22:49PM +0530, Harikrishna Shenoy wrote:
>>>> With the DBANC framework, the connector is no longer initialized in
>>>> bridge_attach() when the display controller sets the
>>>> DRM_BRIDGE_ATTACH_NO_CONNECTOR flag.
>>>> This causes a null pointer dereference in cdns_mhdp_modeset_retry_fn()
>>>> when trying to access &conn->dev->mode_config.mutex.
>>>> Observed on a board where EDID read failed.
>>>> (log: https://gist.github.com/Jayesh2000/233f87f9becdf1e66f1da6fd53f77429)
>>>>
>>>> Patch 1 adds a connector_ptr which takes care of both
>>>> DBANC and !DBANC case by setting the pointer in appropriate hooks
>>>> and checking for pointer validity before accessing the connector.
>>>> Patch 2 adds mode validation hook to bridge fucntions.
>>>> Patch 3 fixes HDCP to work with both DBANC and !DBANC case by
>>>> moving HDCP state handling into the bridge atomic check in line with
>>>> the DBANC model.
>>>> Patches 4,5 do necessary cleanup and alignment for using
>>>> connector pointer.
>>>
>>> It's mentioned several times in your series, and it might be obvious to
>>> you, but documenting what is the "DBANC framework" is would be helpful.
>>> I have no idea what it's about, and it appears that Google doesn't know
>>> either.
>> Yes, I was a bit baffled initially. DRM_BRIDGE_ATTACH_NO_CONNECTOR.
> 
> Oooooh, thanks
> 
>> I think it makes sense to only use "DBANC" if it's first introduced in
>> that patch. So don't have a patch that just uses "DBANC", even if the
>> previous patch did explain what it means. And if there's just one or two
>> "DBANC"s, just spell it out "DRM_BRIDGE_ATTACH_NO_CONNECTOR".
> 
> Yeah, I'd go even further. Acronyms are fun but something being obvious
> is better still. Use the proper flag name every time.
> 
> Maxime

Hi Maxime,

Thanks for pointing this out, will resend the series replacing the 
acronym with proper flag name.

Regards.