Re: [PATCH] drm/amd: DC pull request review

From: Daniel Vetter <daniel.vetter@ffwll.ch>
To: Harry Wentland <harry.wentland@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>,
	Daniel Vetter <daniel.vetter@intel.com>,
	DRI Development <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH] drm/amd: DC pull request review
Date: Thu, 28 Sep 2017 08:28:33 +0200	[thread overview]
Message-ID: <CAKMK7uFd9wMmjeq2hJyeoHJ_M1fTgJmbcRxSG2T1=5XKk8Yq=A@mail.gmail.com> (raw)
In-Reply-To: <24bc04dc-7403-10e3-cbbb-0c05650ed740@amd.com>

On Wed, Sep 27, 2017 at 9:25 PM, Harry Wentland <harry.wentland@amd.com> wrote:
> On 2017-09-27 02:12 PM, Harry Wentland wrote:
>> On 2017-09-27 12:48 PM, Daniel Vetter wrote:
>>> On Wed, Sep 27, 2017 at 6:38 PM, Sean Paul <seanpaul@chromium.org> wrote:
>>>> Any chance we can address the i2c/gpio [re-]implementations as well?
>>>
>>> It's already on the list. Part of this is code that's probably dead,
>>> the other is a bit too much layer cake still left, and the final bits
>>> are not fully using drm helpers for edid parsing and tons of other
>>> stuff. But afaics all the i2c stuff does go through the i2c_adapter
>>> abstraction in a proper fashion, which is at least the foundational
>>> work. The other bits should be all captured in the todo already.
>>
>> There might still be some dead code around i2c stuff. I'll take a look
>> and see what I can rip out.
>>
>
> Looks like there currently are no easy pickings. What we have is:
>
> 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.

I spotted this too, but I'm not aware of any other driver/hw using
this. If it's some standard we might indeed want to move it into the
helpers, but not as a priority. That's why I didn't add it as a todo
item.

All the other bits should be in the TODO already (with my latest patch
at least). You might want to extend/clarify those entries though.

> It would take a bit of time to address all of them. I'll add them on the
> TODO.
>
> 1 & 3 might be a good exercise if someone is looking for things to do.

Yeah, some of this stuff looks like good gsoc/evoc/outreachy stuff.
-Daniel

>
> Harry
>
>
>> If there's any specific function, struct, etc. that's of concern let me
>> know as well.
>>
>> Harry
>>
>>> -Daniel
>>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel