[Intel-gfx] [PATCH 0/4] drm/i915: remove is_cpu_edp()

[Rodrigo Vivi]

Hi Imre,

I just reviewed all patches in this series and saw no issue.
So, in this sense, you and Daniel are free to use "Reviewed-by:
Rodrigo Vivi <rodrigo.vivi at gmail.com>".

However before you move ahead I need to show my concern with the whole idea.
is_cpu_edp is an easy abstraction and provides an easy way to add new
things/restrictions/was/etc like eDP on port D support for HSW.

In the new way code can be more polluted and bugs can be easily added
by forgetting to add a check somewhere when hunting for places like
port_A || vlv ||  to add || (HSW && port_D).

Imagine in some time we have more platforms like valleyview and more
platforms that supports edp on many other different ports.
I really would like to see those checks together in only one place, i.
e., is_cpu_edp().

Cheers,
Rodrigo.

On Thu, May 16, 2013 at 8:40 AM, Imre Deak <imre.deak at intel.com> wrote:
> is_cpu_edp() is equivalent with a port==PORT_A check. There are two
> exceptions (see patch 1 and 2), where we can rewrite things to handle
> ValleyView separately as it's done at other places. With these out of
> the way we can replace is_cpu_edp() with a simpler port check and
> remove is_cpu_edp().
>
> I tested this only on IVB, so before applying we should test it on VLV
> at least.
>
> Imre Deak (4):
>   drm/i915: stop using is_cpu_edp() in intel_disable/post_disable_dp
>   drm/i915: merge VLV eDP and DP AUX clock divider calculation
>   drm/i915: replace is_cpu_edp() with a check for port A
>   drm/i915: remove unused is_cpu_edp()
>
>  drivers/gpu/drm/i915/intel_dp.c |   78 ++++++++++++++++++---------------------
>  1 file changed, 35 insertions(+), 43 deletions(-)
>
> --
> 1.7.10.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br