Re: [PATCH v3 6/9] drm/vc4: hdmi: Convert to common HDMI 2.0 scrambling infrastructure

[Maíra Canal]

Hi Cristian,

On 13/10/26 22:03, Cristian Ciocaltea wrote:
> Replace driver local scrambling logic with the newly introduced common
> HDMI 2.0 scrambling support:
>
> - Advertise source-side scrambling capability by setting supported HDMI
>    version based on the variant's max_pixel_clock.
>
> - Provide minimal .scrambler_{enable,disable} connector callbacks that
>    only toggle the VC5 HDMI_SCRAMBLER_CTL register, while delegating
>    sink-side SCDC programming and periodic status monitoring to
>    drm_connector_hdmi_{enable,disable}_scrambling().
>
> - Drop vc4_hdmi_reset_link() and vc4_hdmi_handle_hotplug(), switching
>    the .detect_ctx() path to drm_atomic_helper_connector_hdmi_hotplug()
>    which internally calls drm_connector_hdmi_sync_scdc() to trigger a
>    CRTC reset on reconnection.
>
> - Drop the local scrambling_work delayed workqueue and scdc_enabled
>    flag, now tracked by the common drm_connector_hdmi layer, as well as
>    the now unused output_{bpc,color_format} driver data members.
>
> No functional change intended.
>
> Acked-by: Maxime Ripard <mripard@xxxxxxxxxx>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@xxxxxxxxxxxxx>
> ---
>   drivers/gpu/drm/vc4/vc4_hdmi.c | 230 +++++------------------------------------
>   drivers/gpu/drm/vc4/vc4_hdmi.h |  21 ----
>   2 files changed, 28 insertions(+), 223 deletions(-)

[...]

>   
> +	/*
> +	 * Since we don't know the state of the controller and its
> +	 * display (if any), let's assume it's always enabled.
> +	 * drm_connector_hdmi_disable_scrambling() will thus run at boot,
> +	 * make sure it's disabled, and avoid any inconsistency.
> +	 */
> +	connector->hdmi.scrambler_enabled = drm_connector_hdmi_scrambler_supported(connector);
> +

I'm okay merging this series to drm-misc-next, but I believe that
Sashiko's report has a point about this chunk. If the display if off or
disconnected, the boot-time disable would never run. Could you please
take a look at it?

Also, while greping the code, one thing caught my attention:
drm_connector_hdmi_disable_scrambling() is called at
vc4_crtc_disable_at_boot() before drm_dev_register(). Does that mean
that connector->status might be connector_status_unknown at that point?
I'm concerned due to the last if-block in
drm_connector_hdmi_disable_scrambling(). Am I mistaken?

Best regards,
- Maíra