diff mbox

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

Message ID 20170928141355.6967-1-harry.wentland@amd.com (mailing list archive)
State New, archived
Headers show

Commit Message

Harry Wentland Sept. 28, 2017, 2:13 p.m. UTC
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@amd.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/amd/display/TODO | 25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

Comments

Alex Deucher Sept. 28, 2017, 4:47 p.m. UTC | #1
> -----Original Message-----

> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf

> Of Harry Wentland

> Sent: Thursday, September 28, 2017 10:14 AM

> To: amd-gfx@lists.freedesktop.org

> Cc: daniel.vetter@ffwll.ch; dri-devel@lists.freedesktop.org;

> seanpaul@chromium.org; Deucher, Alexander; daniel.vetter@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@amd.com>

> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>


Acked-by: Alex Deucher <alexander.deucher@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@lists.freedesktop.org

> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
diff mbox

Patch

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.