[PATCH v2] drm/amd/display: DC I2C review

[Deucher, Alexander]

> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf
> Of Harry Wentland
> Sent: Thursday, September 28, 2017 10:14 AM
> To: amd-gfx at lists.freedesktop.org
> Cc: daniel.vetter at ffwll.ch; dri-devel at lists.freedesktop.org;
> seanpaul at chromium.org; Deucher, Alexander; daniel.vetter at intel.com;
> Wentland, Harry
> Subject: [PATCH v2] drm/amd/display: DC I2C review
> 
> While reviewing I2C in DC identified a few places. Added a couple to the
> TODO list.
> 
> 1) Connector info read
> 
> See get_ext_display_connection_info
> 
> On some boards the connector information has to be read through a
> special I2C channel. This line is only used for this purpose and only on
> driver init.
> 
> 2) SCDC stuff
> 
> This should all be reworked to go through DRM's SCDC code. When this is
> done some unnecessary I2C code can be retired as well.
> 
> 3) Max TMDS clock read
> 
> See dal_ddc_service_i2c_query_dp_dual_mode_adaptor
> 
> This should happen in DRM as well. I haven't checked if there's
> currently functionality in DRM. If not we can propose something.
> 
> 4) HDMI retimer programming
> 
> Some boards have an HDMI retimer that we need to program to pass PHY
> compliance.
> 
> 1 & 3 might be a good exercise if someone is looking for things to do.
> 
> v2: Merge dp_dual_mode_adaptor TODO
> 
> Signed-off-by: Harry Wentland <harry.wentland at amd.com>
> Acked-by: Daniel Vetter <daniel.vetter at ffwll.ch>

Acked-by: Alex Deucher <alexander.deucher at amd.com>

> ---
>  drivers/gpu/drm/amd/display/TODO | 25 ++++++++++---------------
>  1 file changed, 10 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/TODO
> b/drivers/gpu/drm/amd/display/TODO
> index eea645b102a1..46464678f2b3 100644
> --- a/drivers/gpu/drm/amd/display/TODO
> +++ b/drivers/gpu/drm/amd/display/TODO
> @@ -62,20 +62,10 @@ TODOs
>      ~ Daniel Vetter
> 
> 
> -11. Remove existing i2c implementation from DC
> -
> -    "Similar story for i2c, it uses the kernel's i2c code now, but there's
> -    still a full i2c implementation hidden beneath that in
> -    display/dc/i2caux. Kinda not cool, but imo ok if you fix that
> -    post-merging (perhaps by not including any of this in the linux DC
> -    code in the upstream kernel, but as an aux module in your internal
> -    codebase since there you probably need that, same applies to the edid
> -    parsing DC still does. For both cases I assume that the minimal shim
> -    you need on linux (bit banging and edid parsing isn't rocket since) is
> -    a lot less than the glue code to interface with the dc-provided
> -    abstraction."
> -    ~ Daniel Vetter
> -
> +11. Remove dc/i2caux. This folder can be somewhat misleading. It's basically
> an
> +overy complicated HW programming function for sendind and receiving
> i2c/aux
> +commands. We can greatly simplify that and move it into dc/dceXYZ like
> other
> +HW blocks.
> 
>  12. drm_modeset_lock in MST should no longer be needed in recent kernels
>      * Adopt appropriate locking scheme
> @@ -89,7 +79,8 @@ moving all your driver state printing into the various
> atomic_print_state
>  callbacks. There's also plans to expose this stuff in a standard way across all
>  drivers, to make debugging userspace compositors easier across different
> hw.
> 
> -15. Move DP/HDMI dual mode adaptors to drm_dp_dual_mode_helper.c.
> +15. Move DP/HDMI dual mode adaptors to drm_dp_dual_mode_helper.c.
> See
> +dal_ddc_service_i2c_query_dp_dual_mode_adaptor.
> 
>  16. Move to core SCDC helpers (I think those are new since initial DC review).
> 
> @@ -110,3 +101,7 @@ guilty.
>  stuff just isn't up to the challenges either. We need to figure out something
>  that integrates better with DRM and linux debug printing, while not being
>  useless with filtering output. dynamic debug printing might be an option.
> +
> +20. Use kernel i2c device to program HDMI retimer. Some boards have an
> HDMI
> +retimer that we need to program to pass PHY compliance. Currently that's
> +bypassing the i2c device and goes directly to HW. This should be changed.
> --
> 2.11.0
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx