From: Marijn Suijten <marijn.suijten@somainline.org> To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Cc: phone-devel@vger.kernel.org, "Rob Clark" <robdclark@gmail.com>, "Abhinav Kumar" <quic_abhinavk@quicinc.com>, "Vinod Koul" <vkoul@kernel.org>, ~postmarketos/upstreaming@lists.sr.ht, "AngeloGioacchino Del Regno" <angelogioacchino.delregno@somainline.org>, "Konrad Dybcio" <konrad.dybcio@somainline.org>, "Martin Botka" <martin.botka@somainline.org>, "Jami Kettunen" <jami.kettunen@somainline.org>, "Sean Paul" <sean@poorly.run>, "David Airlie" <airlied@gmail.com>, "Daniel Vetter" <daniel@ffwll.ch>, "Stephen Boyd" <swboyd@chromium.org>, "Bjorn Andersson" <andersson@kernel.org>, "Jessica Zhang" <quic_jesszhan@quicinc.com>, "Ville Syrjälä" <ville.syrjala@linux.intel.com>, "Kuogee Hsieh" <quic_khsieh@quicinc.com>, "Jani Nikula" <jani.nikula@intel.com>, sunliming <sunliming@kylinos.cn>, "Sam Ravnborg" <sam@ravnborg.org>, "Haowen Bai" <baihaowen@meizu.com>, "Konrad Dybcio" <konrad.dybcio@linaro.org>, "Loic Poulain" <loic.poulain@linaro.org>, "Vinod Polimera" <quic_vpolimer@quicinc.com>, "Douglas Anderson" <dianders@chromium.org>, "Vladimir Lypak" <vladimir.lypak@gmail.com>, linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org, freedreno@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: Re: [RFC PATCH 3/6] drm/msm/dpu1: Wire up DSC mask for active CTL configuration Date: Wed, 14 Dec 2022 20:30:26 +0100 [thread overview] Message-ID: <20221214193026.dv2fuubysctcvlkg@SoMainline.org> (raw) In-Reply-To: <184d22f1-7ed1-4a67-1c25-9fafeb94db83@linaro.org> On 2022-12-14 20:43:29, Dmitry Baryshkov wrote: > On 14/12/2022 01:22, Marijn Suijten wrote: > > Active CTLs have to configure what DSC block(s) have to be enabled, and > > what DSC block(s) have to be flushed; this value was initialized to zero > > resulting in the necessary register writes to never happen (or would > > write zero otherwise). This seems to have gotten lost in the DSC v4->v5 > > series while refactoring how the combination with merge_3d was handled. > > > > Fixes: 58dca9810749 ("drm/msm/disp/dpu1: Add support for DSC in encoder") > > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> > > --- > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 1 + > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 1 + > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 2 ++ > > 3 files changed, 4 insertions(+) > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c > > index ae28b2b93e69..35791f93c33d 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c > > @@ -61,6 +61,7 @@ static void _dpu_encoder_phys_cmd_update_intf_cfg( > > intf_cfg.intf_mode_sel = DPU_CTL_MODE_SEL_CMD; > > intf_cfg.stream_sel = cmd_enc->stream_sel; > > intf_cfg.mode_3d = dpu_encoder_helper_get_3d_blend_mode(phys_enc); > > + intf_cfg.dsc = dpu_encoder_helper_get_dsc(phys_enc); > > ctl->ops.setup_intf_cfg(ctl, &intf_cfg); > > > > /* setup which pp blk will connect to this intf */ > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c > > index 0f71e8fe7be7..9ee3a7306a5f 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c > > @@ -274,6 +274,7 @@ static void dpu_encoder_phys_vid_setup_timing_engine( > > intf_cfg.intf_mode_sel = DPU_CTL_MODE_SEL_VID; > > intf_cfg.stream_sel = 0; /* Don't care value for video mode */ > > intf_cfg.mode_3d = dpu_encoder_helper_get_3d_blend_mode(phys_enc); > > + intf_cfg.dsc = dpu_encoder_helper_get_dsc(phys_enc); > > if (phys_enc->hw_pp->merge_3d) > > intf_cfg.merge_3d = phys_enc->hw_pp->merge_3d->idx; > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c > > index 7cbcef6efe17..92ddf9995b37 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c > > @@ -209,6 +209,7 @@ static void dpu_encoder_phys_wb_setup_cdp(struct dpu_encoder_phys *phys_enc) > > > > intf_cfg.intf = DPU_NONE; > > intf_cfg.wb = hw_wb->idx; > > + intf_cfg.dsc = dpu_encoder_helper_get_dsc(phys_enc); > > We usually don't have DSC with the writeback, don't we? I am unsure so ended up adding them in writeback regardless. Downstream uses a separate callback to process intf_cfg.dsc instead of going through setup_intf_cfg(). To prevent these from being missed again (in the case of copy&paste), how about instead having some function that sets up intf_cfg with these default values from a phys_enc? That way most of this remains oblivious to the caller. On the same note, that callback on non-DPU_CTL_ACTIVE_CFG hardware doesn't use the intf_cfg.dsc member anyway, but it was again added to keep the blocks somewhat consistent (in case it ever becomes used?). > > if (mode_3d && hw_pp && hw_pp->merge_3d) > > intf_cfg.merge_3d = hw_pp->merge_3d->idx; > > @@ -230,6 +231,7 @@ static void dpu_encoder_phys_wb_setup_cdp(struct dpu_encoder_phys *phys_enc) > > intf_cfg.wb = hw_wb->idx; > > intf_cfg.mode_3d = > > dpu_encoder_helper_get_3d_blend_mode(phys_enc); > > + intf_cfg.dsc = dpu_encoder_helper_get_dsc(phys_enc); > > phys_enc->hw_ctl->ops.setup_intf_cfg(phys_enc->hw_ctl, &intf_cfg); > > } > > } - Marijn
WARNING: multiple messages have this Message-ID (diff)
From: Marijn Suijten <marijn.suijten@somainline.org> To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Cc: Konrad Dybcio <konrad.dybcio@somainline.org>, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org>, phone-devel@vger.kernel.org, Sam Ravnborg <sam@ravnborg.org>, Haowen Bai <baihaowen@meizu.com>, Vinod Koul <vkoul@kernel.org>, Kuogee Hsieh <quic_khsieh@quicinc.com>, Jessica Zhang <quic_jesszhan@quicinc.com>, Jani Nikula <jani.nikula@intel.com>, linux-arm-msm@vger.kernel.org, Abhinav Kumar <quic_abhinavk@quicinc.com>, Stephen Boyd <swboyd@chromium.org>, Martin Botka <martin.botka@somainline.org>, ~postmarketos/upstreaming@lists.sr.ht, Sean Paul <sean@poorly.run>, Loic Poulain <loic.poulain@linaro.org>, Jami Kettunen <jami.kettunen@somainline.org>, Bjorn Andersson <andersson@kernel.org>, Vladimir Lypak <vladimir.lypak@gmail.com>, Douglas Anderson <dianders@chromium.org>, Konrad Dybcio <konrad.dybcio@linaro.org>, sunliming <sunliming@kylinos.cn>, freedreno@lists.freedesktop.org, Vinod Polimera <quic_vpolimer@quicinc.com> Subject: Re: [RFC PATCH 3/6] drm/msm/dpu1: Wire up DSC mask for active CTL configuration Date: Wed, 14 Dec 2022 20:30:26 +0100 [thread overview] Message-ID: <20221214193026.dv2fuubysctcvlkg@SoMainline.org> (raw) In-Reply-To: <184d22f1-7ed1-4a67-1c25-9fafeb94db83@linaro.org> On 2022-12-14 20:43:29, Dmitry Baryshkov wrote: > On 14/12/2022 01:22, Marijn Suijten wrote: > > Active CTLs have to configure what DSC block(s) have to be enabled, and > > what DSC block(s) have to be flushed; this value was initialized to zero > > resulting in the necessary register writes to never happen (or would > > write zero otherwise). This seems to have gotten lost in the DSC v4->v5 > > series while refactoring how the combination with merge_3d was handled. > > > > Fixes: 58dca9810749 ("drm/msm/disp/dpu1: Add support for DSC in encoder") > > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> > > --- > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 1 + > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 1 + > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 2 ++ > > 3 files changed, 4 insertions(+) > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c > > index ae28b2b93e69..35791f93c33d 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c > > @@ -61,6 +61,7 @@ static void _dpu_encoder_phys_cmd_update_intf_cfg( > > intf_cfg.intf_mode_sel = DPU_CTL_MODE_SEL_CMD; > > intf_cfg.stream_sel = cmd_enc->stream_sel; > > intf_cfg.mode_3d = dpu_encoder_helper_get_3d_blend_mode(phys_enc); > > + intf_cfg.dsc = dpu_encoder_helper_get_dsc(phys_enc); > > ctl->ops.setup_intf_cfg(ctl, &intf_cfg); > > > > /* setup which pp blk will connect to this intf */ > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c > > index 0f71e8fe7be7..9ee3a7306a5f 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c > > @@ -274,6 +274,7 @@ static void dpu_encoder_phys_vid_setup_timing_engine( > > intf_cfg.intf_mode_sel = DPU_CTL_MODE_SEL_VID; > > intf_cfg.stream_sel = 0; /* Don't care value for video mode */ > > intf_cfg.mode_3d = dpu_encoder_helper_get_3d_blend_mode(phys_enc); > > + intf_cfg.dsc = dpu_encoder_helper_get_dsc(phys_enc); > > if (phys_enc->hw_pp->merge_3d) > > intf_cfg.merge_3d = phys_enc->hw_pp->merge_3d->idx; > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c > > index 7cbcef6efe17..92ddf9995b37 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c > > @@ -209,6 +209,7 @@ static void dpu_encoder_phys_wb_setup_cdp(struct dpu_encoder_phys *phys_enc) > > > > intf_cfg.intf = DPU_NONE; > > intf_cfg.wb = hw_wb->idx; > > + intf_cfg.dsc = dpu_encoder_helper_get_dsc(phys_enc); > > We usually don't have DSC with the writeback, don't we? I am unsure so ended up adding them in writeback regardless. Downstream uses a separate callback to process intf_cfg.dsc instead of going through setup_intf_cfg(). To prevent these from being missed again (in the case of copy&paste), how about instead having some function that sets up intf_cfg with these default values from a phys_enc? That way most of this remains oblivious to the caller. On the same note, that callback on non-DPU_CTL_ACTIVE_CFG hardware doesn't use the intf_cfg.dsc member anyway, but it was again added to keep the blocks somewhat consistent (in case it ever becomes used?). > > if (mode_3d && hw_pp && hw_pp->merge_3d) > > intf_cfg.merge_3d = hw_pp->merge_3d->idx; > > @@ -230,6 +231,7 @@ static void dpu_encoder_phys_wb_setup_cdp(struct dpu_encoder_phys *phys_enc) > > intf_cfg.wb = hw_wb->idx; > > intf_cfg.mode_3d = > > dpu_encoder_helper_get_3d_blend_mode(phys_enc); > > + intf_cfg.dsc = dpu_encoder_helper_get_dsc(phys_enc); > > phys_enc->hw_ctl->ops.setup_intf_cfg(phys_enc->hw_ctl, &intf_cfg); > > } > > } - Marijn
next prev parent reply other threads:[~2022-12-14 19:30 UTC|newest] Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-12-13 23:22 [RFC PATCH 0/6] drm/msm: DSC Electric Boogaloo for sm8[12]50 Marijn Suijten 2022-12-13 23:22 ` Marijn Suijten 2022-12-13 23:22 ` [RFC PATCH 1/6] drm/msm/dpu1: Implement DSC binding to PP block for CTL V1 Marijn Suijten 2022-12-13 23:22 ` Marijn Suijten 2022-12-14 18:40 ` Dmitry Baryshkov 2022-12-14 18:40 ` Dmitry Baryshkov 2022-12-16 20:51 ` Abhinav Kumar 2022-12-16 20:51 ` Abhinav Kumar 2022-12-13 23:22 ` [RFC PATCH 2/6] drm/msm/dpu1: Add DSC config for sm8150 and sm8250 Marijn Suijten 2022-12-13 23:22 ` Marijn Suijten 2022-12-14 18:42 ` Dmitry Baryshkov 2022-12-14 18:42 ` Dmitry Baryshkov 2022-12-16 20:53 ` Abhinav Kumar 2022-12-16 20:53 ` Abhinav Kumar 2022-12-13 23:22 ` [RFC PATCH 3/6] drm/msm/dpu1: Wire up DSC mask for active CTL configuration Marijn Suijten 2022-12-13 23:22 ` Marijn Suijten 2022-12-14 18:43 ` Dmitry Baryshkov 2022-12-14 18:43 ` Dmitry Baryshkov 2022-12-14 19:30 ` Marijn Suijten [this message] 2022-12-14 19:30 ` Marijn Suijten 2022-12-15 1:08 ` Dmitry Baryshkov 2022-12-16 22:20 ` Abhinav Kumar 2022-12-20 22:32 ` Marijn Suijten 2022-12-20 22:32 ` Marijn Suijten 2022-12-13 23:22 ` [RFC PATCH 4/6] drm/msm/dsi: Use DSC slice(s) packet size to compute word count Marijn Suijten 2022-12-13 23:22 ` Marijn Suijten 2022-12-14 18:52 ` Dmitry Baryshkov 2022-12-14 18:52 ` Dmitry Baryshkov 2022-12-16 23:01 ` Abhinav Kumar 2022-12-16 23:01 ` Abhinav Kumar 2022-12-13 23:22 ` [RFC PATCH 5/6] drm/msm/dsi: Flip greater-than check for slice_count and slice_per_intf Marijn Suijten 2022-12-13 23:22 ` Marijn Suijten 2022-12-14 0:02 ` Konrad Dybcio 2022-12-14 0:02 ` Konrad Dybcio 2022-12-14 8:38 ` Marijn Suijten 2022-12-14 8:38 ` Marijn Suijten 2022-12-14 18:53 ` Dmitry Baryshkov 2022-12-14 18:53 ` Dmitry Baryshkov 2022-12-17 0:31 ` Abhinav Kumar 2022-12-17 0:31 ` Abhinav Kumar 2022-12-13 23:22 ` [RFC PATCH 6/6] drm/msm/dpu: Disallow unallocated (DSC) resources to be returned Marijn Suijten 2022-12-13 23:22 ` Marijn Suijten 2022-12-14 18:56 ` Dmitry Baryshkov 2022-12-14 18:56 ` Dmitry Baryshkov 2022-12-14 19:31 ` Marijn Suijten 2022-12-14 19:31 ` Marijn Suijten 2022-12-14 18:40 ` [RFC PATCH 0/6] drm/msm: DSC Electric Boogaloo for sm8[12]50 Dmitry Baryshkov 2022-12-14 18:40 ` Dmitry Baryshkov 2022-12-14 19:23 ` Marijn Suijten 2022-12-14 19:23 ` Marijn Suijten 2022-12-15 0:52 ` Dmitry Baryshkov 2022-12-20 22:35 ` Marijn Suijten 2022-12-20 22:35 ` Marijn Suijten
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20221214193026.dv2fuubysctcvlkg@SoMainline.org \ --to=marijn.suijten@somainline.org \ --cc=airlied@gmail.com \ --cc=andersson@kernel.org \ --cc=angelogioacchino.delregno@somainline.org \ --cc=baihaowen@meizu.com \ --cc=daniel@ffwll.ch \ --cc=dianders@chromium.org \ --cc=dmitry.baryshkov@linaro.org \ --cc=dri-devel@lists.freedesktop.org \ --cc=freedreno@lists.freedesktop.org \ --cc=jami.kettunen@somainline.org \ --cc=jani.nikula@intel.com \ --cc=konrad.dybcio@linaro.org \ --cc=konrad.dybcio@somainline.org \ --cc=linux-arm-msm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=loic.poulain@linaro.org \ --cc=martin.botka@somainline.org \ --cc=phone-devel@vger.kernel.org \ --cc=quic_abhinavk@quicinc.com \ --cc=quic_jesszhan@quicinc.com \ --cc=quic_khsieh@quicinc.com \ --cc=quic_vpolimer@quicinc.com \ --cc=robdclark@gmail.com \ --cc=sam@ravnborg.org \ --cc=sean@poorly.run \ --cc=sunliming@kylinos.cn \ --cc=swboyd@chromium.org \ --cc=ville.syrjala@linux.intel.com \ --cc=vkoul@kernel.org \ --cc=vladimir.lypak@gmail.com \ --cc=~postmarketos/upstreaming@lists.sr.ht \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.