Re: [Freedreno] [PATCH v1 2/3] drm/msm/dpu: retrieve DSI DSC struct at atomic_check()

From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
To: Kuogee Hsieh <quic_khsieh@quicinc.com>
Cc: freedreno@lists.freedesktop.org,
	"open list:DRM DRIVER FOR MSM ADRENO GPU" 
	<dri-devel@lists.freedesktop.org>,
	Rob Clark <robdclark@gmail.com>, Sean Paul <sean@poorly.run>,
	Stephen Boyd <swboyd@chromium.org>,
	Douglas Anderson <dianders@chromium.org>,
	Vinod Koul <vkoul@kernel.org>, Daniel Vetter <daniel@ffwll.ch>,
	Dave Airlie <airlied@gmail.com>, Andy Gross <agross@kernel.org>,
	Bjorn Andersson <andersson@kernel.org>,
	Abhinav Kumar <quic_abhinavk@quicinc.com>,
	Jessica Zhang <quic_jesszhan@quicinc.com>,
	Sankeerth Billakanti <quic_sbillaka@quicinc.com>,
	Marijn Suijten <marijn.suijten@somainline.org>,
	"open list:DRM DRIVER FOR MSM ADRENO GPU" 
	<linux-arm-msm@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [Freedreno] [PATCH v1 2/3] drm/msm/dpu: retrieve DSI DSC struct at atomic_check()
Date: Sat, 3 Jun 2023 00:05:27 +0300	[thread overview]
Message-ID: <CAA8EJpp0aWd2i4vkbURgDx+s99KNZYeEkd9RK0+fP+QfzDgZKg@mail.gmail.com> (raw)
In-Reply-To: <32aa41ee-4ab0-0915-a77f-5b0d6874b3e1@quicinc.com>

Generic note: please use reply-to-all instead of any other options to
answer the email. You have dropped all recipients (except the
freedreno@) in the message
<d1a320c4-d851-ba75-ef7b-80dc369d1cfd@quicinc.com> (and it was left
unnoticed).

On Fri, 2 Jun 2023 at 20:00, Kuogee Hsieh <quic_khsieh@quicinc.com> wrote:
> >> There is one option which is keep current
> >>
> >> 1) keep struct drm_dsc_config *msm_dsi_get_dsc_config(struct msm_dsi
> >> *msm_dsi) at dsi.c
> >>
> >> 2) use  struct msm_display_info *disp_info saved at dpu_enc to locate
> >> struct msm_dsi from priv->dsi[] list (see item #3)
> >>
> >> 3)  info.dsc = msm_dsi_get_dsc_config(priv->dsi[info.h_tile_instance[0]]);
> >>
> >> 4) ballistically, keep original code but move  info.dsc =
> >> msm_dsi_get_dsc_config(priv->dsi[i]); to other place sush as
> >> atomic_check() and atomic_enable().
> >>
> > 5) leave drm_dsc_config handling as is, update the dsc config from the
> > DP driver as suitable. If DSC is not supported, set
> > dsc->dsc_version_major = 0 and dsc->dsc_version_minor = 0 on the DP
> > side. In DPU driver verify that dsc->dsc_version_major/_minor != 0.
>
> I am confusing with item 5)
>
> Currently, msm_dsi_get_dsc_config() of dsi side return dsc pointer if
> dsc enabled and NULL if dsc not enabled.
>
> Should checking dsc == NULL is good enough to differentiate between dsc
> is supported and not supported?

This is called a "shared memory area". Instead of either providing a
dynamic data pointer, one can provide a pointer to the static area
which is filled by DP or DSI. If there is no DSC available, one flags
'data not valid' by setting major,minor to 0.

>
> Why need to set both dsc->dsc_version_major = 0 and
> dsc->dsc_version_minor = 0 for dsc is not supported?

6) Another option (which is more in style of what is done in the
vendor kernel, if I'm not mistaken):

Enhance struct drm_display_mode to contain a pointer to the DSC
config. Use this pointer to check whether DSC should be enabled for
the particular mode or not. The panels with the static DSC
configuration can use a static data pointer.

-- 
With best wishes
Dmitry