On Tue, Oct 16, 2018 at 03:01:28PM -0700, Paulo Zanoni wrote:
> Its control flow is not as easy to follow as it could be. We recently
> even had a double register write that went unnoticed until commit
> 9e44b180f81b ("drm/i915: don't write PLANE_BUF_CFG twice every time")
> fixed it. The return statement in the middle along with the fact that
> it's on a void function really doesn't help readability IMHO.
>
> Refactor the function so that the first level of checks is per
> platform and the second level is for planar planes. IMHO that makes
> the code much more readable.
>
> Requested-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_pm.c | 33 ++++++++++++++++++++-------------
> 1 file changed, 20 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 7fd344b81d66..f388bfa99a97 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5033,21 +5033,28 @@ static void skl_write_plane_wm(struct intel_crtc *intel_crtc,
> skl_write_wm_level(dev_priv, PLANE_WM_TRANS(pipe, plane_id),
> &wm->trans_wm);
>
> - /* FIXME: add proper NV12 support for ICL. */
> - if (INTEL_GEN(dev_priv) >= 11)
> - return skl_ddb_entry_write(dev_priv,
> - PLANE_BUF_CFG(pipe, plane_id),
> - &ddb->plane[pipe][plane_id]);
> - if (wm->is_planar) {
> - skl_ddb_entry_write(dev_priv, PLANE_BUF_CFG(pipe, plane_id),
> - &ddb->uv_plane[pipe][plane_id]);
> + if (INTEL_GEN(dev_priv) >= 11) {
> + /* FIXME: add proper NV12 support for ICL. */
> + if (wm->is_planar)
> + DRM_ERROR("No DDB support for planar formats yet\n");
> +
> skl_ddb_entry_write(dev_priv,
> - PLANE_NV12_BUF_CFG(pipe, plane_id),
> - &ddb->plane[pipe][plane_id]);
> + PLANE_BUF_CFG(pipe, plane_id),
> + &ddb->plane[pipe][plane_id]);
> } else {
> - skl_ddb_entry_write(dev_priv, PLANE_BUF_CFG(pipe, plane_id),
> - &ddb->plane[pipe][plane_id]);
> - I915_WRITE(PLANE_NV12_BUF_CFG(pipe, plane_id), 0x0);
> + if (wm->is_planar) {
> + skl_ddb_entry_write(dev_priv,
> + PLANE_BUF_CFG(pipe, plane_id),
> + &ddb->uv_plane[pipe][plane_id]);
> + skl_ddb_entry_write(dev_priv,
> + PLANE_NV12_BUF_CFG(pipe, plane_id),
> + &ddb->plane[pipe][plane_id]);
> + } else {
> + skl_ddb_entry_write(dev_priv,
> + PLANE_BUF_CFG(pipe, plane_id),
> + &ddb->plane[pipe][plane_id]);
> + I915_WRITE(PLANE_NV12_BUF_CFG(pipe, plane_id), 0x0);
> + }
> }
> }
>
> --
> 2.14.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
