Re: [Intel-gfx] [PATCH 02/15] drm/amdgpu: use drm_* functions instead of duplicated code in amdgpu driver

Claudio Suarez
2021-10-16T10:16:06Z

On Fri, Oct 15, 2021 at 11:14:54AM -0400, Harry Wentland wrote:
>
>
> On 2021-10-15 07:37, Claudio Suarez wrote:
> > a) Once EDID is parsed, the monitor HDMI support information is available
> > through drm_display_info.is_hdmi. The amdgpu driver still calls
> > drm_detect_hdmi_monitor() to retrieve the same information, which
> > is less efficient. Change to drm_display_info.is_hdmi
> >
> > This is a TODO task in Documentation/gpu/todo.rst
> >
> > b) drm_display_info is updated by drm_get_edid() or
> > drm_connector_update_edid_property(). In the amdgpu driver it is almost
> > always updated when the edid is read in amdgpu_connector_get_edid(),
> > but not always. Change amdgpu_connector_get_edid() and
> > amdgpu_connector_free_edid() to keep drm_display_info updated. This allows a)
> > to work properly.
> >
> > c) Use drm_edid_get_monitor_name() instead of duplicating the code that
> > parses the EDID in dm_helpers_parse_edid_caps()
> >
> > Also, remove the unused "struct dc_context *ctx" parameter in
> > dm_helpers_parse_edid_caps()
> >
>
> Thanks for this work.
>
> The fact that you listed three separate changes in this commit
> is a clear indication that this patch should be three separate
> patches instead. Separating the functional bits from the straight
> refactor will help with bisection if this leads to a regression.
>
> All changes look reasonable to me, though. With this patch split
> into three patches in the sequence (b), (c), then (a) this is
> Reviewed-by: Harry Wentland <harry.wentland@amd.com>

Ok, thanks. I'll send three patches.

BR
Claudio Suarez