Re: [v2] msm: disp: dpu1: add support to access hw irqs regs depending on revision

From: Stephen Boyd <swboyd@chromium.org>
To: Shubhashree Dhar <dhar@codeaurora.org>,
	devicetree@vger.kernel.org, dri-devel@lists.freedesktop.org,
	freedreno@lists.freedesktop.org, linux-arm-msm@vger.kernel.org
Cc: Shubhashree Dhar <dhar@codeaurora.org>,
	linux-kernel@vger.kernel.org, robdclark@gmail.com,
	seanpaul@chromium.org, hoegsberg@chromium.org,
	abhinavk@codeaurora.org, jsanka@codeaurora.org,
	chandanu@codeaurora.org, nganji@codeaurora.org
Subject: Re: [v2] msm: disp: dpu1: add support to access hw irqs regs depending on revision
Date: Thu, 14 Nov 2019 09:29:40 -0800	[thread overview]
Message-ID: <5dcd8f05.1c69fb81.bdd4.2b0a@mx.google.com> (raw)
In-Reply-To: <1573710976-27551-1-git-send-email-dhar@codeaurora.org>

Quoting Shubhashree Dhar (2019-11-13 21:56:16)
> Current code assumes that all the irqs registers offsets can be
> accessed in all the hw revisions; this is not the case for some
> targets that should not access some of the irq registers.

What happens if we read the irq registers that we "should not access"?
Does the system reset? It would be easier to make those registers return
0 when read indicating no interrupt and ignore writes so that everything
keeps working without having to skip registers.

> This change adds the support to selectively remove the irqs that
> are not supported in some of the hw revisions.
> 
> Change-Id: I6052b8237b703a1a9edd53893e04f7bd72223da1

Please remove these before sending upstream.

> Signed-off-by: Shubhashree Dhar <dhar@codeaurora.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c    |  1 +
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h    |  3 +++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 22 +++++++++++++++++-----
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.h |  1 +
>  4 files changed, 22 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> index ec76b868..def8a3f 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> @@ -646,6 +646,7 @@ struct dpu_perf_cfg {
>   * @dma_formats        Supported formats for dma pipe
>   * @cursor_formats     Supported formats for cursor pipe
>   * @vig_formats        Supported formats for vig pipe
> + * @mdss_irqs          Bitmap with the irqs supported by the target

Hmm pretty sure there needs to be a colon so that kernel-doc can match
this but maybe I'm wrong.

>   */
>  struct dpu_mdss_cfg {
>         u32 hwversion;