Re: [PATCH] drm/msm/dp: fix the max supported bpp logic
On Fri, 26 Jul 2024 at 01:04, Abhinav Kumar wrote:
>
> Currently the DP driver hard-codes the max supported bpp to 30.
> This is incorrect because the number of lanes and max data rate
> supported by the lanes need to be taken into account.
>
> Replace the hardcoded limit with the appropriate math which accounts
> for the accurate number of lanes and max data rate.
>
> Fixes: c943b4948b58 ("drm/msm/dp: add displayPort driver support")
> Signed-off-by: Abhinav Kumar
> ---
> drivers/gpu/drm/msm/dp/dp_panel.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_panel.c
> b/drivers/gpu/drm/msm/dp/dp_panel.c
> index a916b5f3b317..56ce5e4008f8 100644
> --- a/drivers/gpu/drm/msm/dp/dp_panel.c
> +++ b/drivers/gpu/drm/msm/dp/dp_panel.c
> @@ -397,6 +397,7 @@ int dp_panel_init_panel_info(struct dp_panel *dp_panel)
> {
> struct drm_display_mode *drm_mode;
> struct dp_panel_private *panel;
> + u32 max_supported_bpp;
>
> drm_mode = _panel->dp_mode.drm_mode;
>
> @@ -423,8 +424,10 @@ int dp_panel_init_panel_info(struct dp_panel *dp_panel)
> drm_mode->clock);
> drm_dbg_dp(panel->drm_dev, "bpp = %d\n", dp_panel->dp_mode.bpp);
>
> - dp_panel->dp_mode.bpp = max_t(u32, 18,
> - min_t(u32, dp_panel->dp_mode.bpp, 30));
> + max_supported_bpp = dp_panel_get_mode_bpp(dp_panel,
> dp_panel->dp_mode.bpp,
> +
> dp_panel->dp_mode.drm_mode.clock);
> + dp_panel->dp_mode.bpp = max_t(u32, 18, max_supported_bpp);
I think that in mode_valid() the driver should filter out modes that
result in BPP being less than 18. Then the max_t can be dropped
completely.
Nevertheless this indeed fixes an issue with the screen corruption,
this is great!
Tested-by: Dmitry Baryshkov # SM8350-HDK
> +
> drm_dbg_dp(panel->drm_dev, "updated bpp = %d\n",
> dp_panel->dp_mode.bpp);
>
> --
> 2.44.0
>
--
With best wishes
Dmitry
Re: [PATCH] phy: qcom: com-qmp-combo: fix swing and pre-emphasis table for sm8350
On Fri, 26 Jul 2024 at 01:06, Abhinav Kumar wrote:
>
> Fix the voltage swing and pre-emphasis tables for sm8350 as the current
> one do not match the hardware docs.
>
> Fixes: ef14aff107bd ("phy: qcom: com-qmp-combo: add SM8350 & SM8450 support")
> Signed-off-by: Abhinav Kumar
> ---
> drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
Reviewed-by: Dmitry Baryshkov
Tested-by: Dmitry Baryshkov # SM8350-HDK
--
With best wishes
Dmitry
Re: [PATCH] drm/msm/dp: reset the link phy params before link training
On Fri, 26 Jul 2024 at 01:05, Abhinav Kumar wrote:
>
> Before re-starting link training reset the link phy params namely
> the pre-emphasis and voltage swing levels otherwise the next
> link training begins at the previously cached levels which can result
> in link training failures.
>
> Fixes: 8ede2ecc3e5e ("drm/msm/dp: Add DP compliance tests on Snapdragon
> Chipsets")
> Signed-off-by: Abhinav Kumar
Reviewed-by: Dmitry Baryshkov
Tested-by: Dmitry Baryshkov # SM8350-HDK
--
With best wishes
Dmitry
Re: [PATCH v5 4/4] arm64: dts: qcom: add HDMI nodes for msm8998
On Tue, 23 Jul 2024 at 15:57, Marc Gonzalez wrote: > > On 23/07/2024 13:45, Konrad Dybcio wrote: > > > On 23.07.2024 11:59 AM, Dmitry Baryshkov wrote: > > > >> On Tue, 23 Jul 2024 at 12:48, Marc Gonzalez wrote: > >> > >>> On 16/07/2024 18:37, Dmitry Baryshkov wrote: > >>> > >>>> No, that's fine. It is the SMMU issue that Konrad has been asking you > >>>> to take a look at. > >>> > >>> Context: > >>> > >>> [4.911422] arm-smmu cd0.iommu: FSR= 0402 [Format=2 TF], > >>> SID=0x0 > >>> [4.923353] arm-smmu cd0.iommu: FSYNR0 = 0021 [S1CBNDX=0 PNU > >>> PLVL=1] > >>> [4.927893] arm-smmu cd0.iommu: FSR= 0402 [Format=2 TF], > >>> SID=0x0 > >>> [4.941928] arm-smmu cd0.iommu: FSYNR0 = 0021 [S1CBNDX=0 PNU > >>> PLVL=1] > >>> [4.944438] arm-smmu cd0.iommu: FSR= 0402 [Format=2 TF], > >>> SID=0x0 > >>> [4.956013] arm-smmu cd0.iommu: FSYNR0 = 0021 [S1CBNDX=0 PNU > >>> PLVL=1] > >>> [4.961055] arm-smmu cd0.iommu: FSR= 0402 [Format=2 TF], > >>> SID=0x0 > >>> [4.974565] arm-smmu cd0.iommu: FSYNR0 = 0021 [S1CBNDX=0 PNU > >>> PLVL=1] > >>> [4.977628] arm-smmu cd0.iommu: FSR= 0402 [Format=2 TF], > >>> SID=0x0 > >>> [4.989670] arm-smmu cd0.iommu: FSYNR0 = 0021 [S1CBNDX=0 PNU > >>> PLVL=1] > >>> > >>> > >>> As I mentioned, I don't think I've ever seen issues from cd0.iommu > >>> on my board. > >> > >> Interestingly enough, I can also see iommu errors during WiFi startup > >> / shutdown on msm8998 / miix630. This leads me to thinking that it > >> well might be that there is a missing quirk in the iommu driver. > >> > >>> I can test a reboot loop for a few hours, to see if anything shows up. > >> > >> Yes, please. > > > > Yeah I do trust you Marc that it actually works for you and I'm not > > gonna delay this series because of that, but please go ahead and > > reboot-loop your board > > > > 8998/660 is """famous""" for it's iommu problems > > [ 20.501062] arm-smmu 16c.iommu: Unhandled context fault: fsr=0x402, > iova=0x, fsynr=0x1, cbfrsynra=0x1900, cb=0 I think 0x1900 is WiFi. > > I get the above warning pretty reliably. > I don't think it's related to the issue(s) you mentioned. > System just keeps plodding along. > > Regards > -- With best wishes Dmitry
Re: [PATCH v5 4/4] arm64: dts: qcom: add HDMI nodes for msm8998
On Tue, 23 Jul 2024 at 12:48, Marc Gonzalez wrote: > > On 16/07/2024 18:37, Dmitry Baryshkov wrote: > > > No, that's fine. It is the SMMU issue that Konrad has been asking you > > to take a look at. > > Context: > > [4.911422] arm-smmu cd0.iommu: FSR= 0402 [Format=2 TF], > SID=0x0 > [4.923353] arm-smmu cd0.iommu: FSYNR0 = 0021 [S1CBNDX=0 PNU > PLVL=1] > [4.927893] arm-smmu cd0.iommu: FSR= 0402 [Format=2 TF], > SID=0x0 > [4.941928] arm-smmu cd0.iommu: FSYNR0 = 0021 [S1CBNDX=0 PNU > PLVL=1] > [4.944438] arm-smmu cd0.iommu: FSR= 0402 [Format=2 TF], > SID=0x0 > [4.956013] arm-smmu cd0.iommu: FSYNR0 = 0021 [S1CBNDX=0 PNU > PLVL=1] > [4.961055] arm-smmu cd0.iommu: FSR= 0402 [Format=2 TF], > SID=0x0 > [4.974565] arm-smmu cd0.iommu: FSYNR0 = 0021 [S1CBNDX=0 PNU > PLVL=1] > [4.977628] arm-smmu cd0.iommu: FSR= 0402 [Format=2 TF], > SID=0x0 > [4.989670] arm-smmu cd0.iommu: FSYNR0 = 0021 [S1CBNDX=0 PNU > PLVL=1] > > > As I mentioned, I don't think I've ever seen issues from cd0.iommu > on my board. Interestingly enough, I can also see iommu errors during WiFi startup / shutdown on msm8998 / miix630. This leads me to thinking that it well might be that there is a missing quirk in the iommu driver. > > I can test a reboot loop for a few hours, to see if anything shows up. Yes, pleas. -- With best wishes Dmitry
Re: [PATCH v5 08/16] drm/msm/dpu: drop msm_format from struct dpu_hw_fmt_layout
On Wed, 17 Jul 2024 at 23:15, Abhinav Kumar wrote:
>
>
>
> On 6/24/2024 2:13 PM, Dmitry Baryshkov wrote:
> > The struct dpu_hw_fmt_layout defines hardware data layout (addresses,
> > sizes and pitches. Drop format field from this structure as it's not a
> Missing closing brace ")" here?
>
> > part of the data layout.
> >
>
> Its a bit subjective IMO whether you consider format as part of hardware
> data layout or not. Registers do have format bitfields too so I am
> somewhat unsure if this change is really needed.
It's not a part of the data buffer layout (num_planes, sizes, pitches
and offsets) - the items that are defined by struct dpu_hw_fmt_layout.
As such there is no need to store it in the structure. When necessary
we can always get it from the framebuffer itself.
>
> > Signed-off-by: Dmitry Baryshkov
> > ---
> > .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c| 31
> > +++---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c| 23
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h| 2 --
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 4 +--
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h | 3 ++-
> > 5 files changed, 25 insertions(+), 38 deletions(-)
> >
>
>
>
> > @@ -318,15 +318,10 @@ static void dpu_encoder_phys_wb_setup(
> > {
> > struct dpu_hw_wb *hw_wb = phys_enc->hw_wb;
> > struct drm_display_mode mode = phys_enc->cached_mode;
> > - struct drm_framebuffer *fb = NULL;
> > struct dpu_encoder_phys_wb *wb_enc = to_dpu_encoder_phys_wb(phys_enc);
> > - struct drm_writeback_job *wb_job;
> > const struct msm_format *format;
> > - const struct msm_format *dpu_fmt;
> >
> > - wb_job = wb_enc->wb_job;
> > format = msm_framebuffer_format(wb_enc->wb_job->fb);
> > - dpu_fmt = mdp_get_format(_enc->dpu_kms->base,
> > format->pixel_format, wb_job->fb->modifier);
> >
>
> This is interesting. I wonder why I just didnt use format directly that
> time itself :)
>
> Maybe I was thinking that mdp_get_format() will also match the modifiers
> and return the corresponding msm_format.
>
> > DPU_DEBUG("[mode_set:%d, \"%s\",%d,%d]\n",
> > hw_wb->idx - WB_0, mode.name,
> > @@ -338,9 +333,9 @@ static void dpu_encoder_phys_wb_setup(
> >
> > dpu_encoder_phys_wb_set_qos(phys_enc);
> >
> > - dpu_encoder_phys_wb_setup_fb(phys_enc, fb);
> > + dpu_encoder_phys_wb_setup_fb(phys_enc, format);
> >
> > - dpu_encoder_helper_phys_setup_cdm(phys_enc, dpu_fmt,
> > CDM_CDWN_OUTPUT_WB);
> > + dpu_encoder_helper_phys_setup_cdm(phys_enc, format,
> > CDM_CDWN_OUTPUT_WB);
> >
> > dpu_encoder_phys_wb_setup_ctl(phys_enc);
> > }
> > @@ -584,14 +579,6 @@ static void dpu_encoder_phys_wb_prepare_wb_job(struct
> > dpu_encoder_phys *phys_enc
> >
> > format = msm_framebuffer_format(job->fb);
> >
> > - wb_cfg->dest.format = mdp_get_format(_enc->dpu_kms->base,
> > - format->pixel_format,
> > job->fb->modifier);
> > - if (!wb_cfg->dest.format) {
> > - /* this error should be detected during atomic_check */
> > - DPU_ERROR("failed to get format %p4cc\n",
> > >pixel_format);
> > - return;
> > - }
> > -
> > ret = dpu_format_populate_layout(aspace, job->fb, _cfg->dest);
> > if (ret) {
> > DPU_DEBUG("failed to populate layout %d\n", ret);
--
With best wishes
Dmitry
Re: [PATCH v4 2/5] drm/drm_property: require DRM_MODE_PROP_IMMUTABLE for single-value props
On Wed, Jul 17, 2024 at 03:42:46PM GMT, Maxime Ripard wrote: > Hi, > > On Mon, Jul 15, 2024 at 09:33:02AM GMT, Dmitry Baryshkov wrote: > > Document that DRM_MODE_PROP_IMMUTABLE must be set for the properties > > that are immutable by definition - e.g. ranges with min == max or enums > > with a single value. This matches the behaviour of the IGT tests, see > > kms_properties.c / validate_range_prop(), validate_enum_prop(), > > validate_bitmask_prop(). > > > > Signed-off-by: Dmitry Baryshkov > > We had a discussion yesterday about it on IRC with Sima, Simon and > Xaver. > > https://oftc.irclog.whitequark.org/dri-devel/2024-07-16#33374622; > > The conclusion was that it would create an inconsistency between drivers > on whether a given property is immutable or not, which will lead to more > troubles for userspace. > > It's not clear why Ville added that check in the first place, so the > best course of action is to remove the IGT test and get the discussion > started there. Ack, I'll work on removing those tests later today. -- With best wishes Dmitry
Re: [PATCH 5/5] drm/msm/dpu: rate limit snapshot capture for mmu faults
On Wed, 17 Jul 2024 at 02:15, Abhinav Kumar wrote: > > > > On 7/16/2024 4:10 PM, Dmitry Baryshkov wrote: > > On Wed, 17 Jul 2024 at 01:43, Abhinav Kumar > > wrote: > >> > >> > >> > >> On 7/16/2024 2:50 PM, Rob Clark wrote: > >>> On Tue, Jul 16, 2024 at 2:45 PM Abhinav Kumar > >>> wrote: > >>>> > >>>> > >>>> > >>>> On 7/15/2024 12:51 PM, Rob Clark wrote: > >>>>> On Mon, Jul 1, 2024 at 12:43 PM Dmitry Baryshkov > >>>>> wrote: > >>>>>> > >>>>>> On Fri, Jun 28, 2024 at 02:48:47PM GMT, Abhinav Kumar wrote: > >>>>>>> There is no recovery mechanism in place yet to recover from mmu > >>>>>>> faults for DPU. We can only prevent the faults by making sure there > >>>>>>> is no misconfiguration. > >>>>>>> > >>>>>>> Rate-limit the snapshot capture for mmu faults to once per > >>>>>>> msm_kms_init_aspace() as that should be sufficient to capture > >>>>>>> the snapshot for debugging otherwise there will be a lot of > >>>>>>> dpu snapshots getting captured for the same fault which is > >>>>>>> redundant and also might affect capturing even one snapshot > >>>>>>> accurately. > >>>>>> > >>>>>> Please squash this into the first patch. There is no need to add code > >>>>>> with a known defficiency. > >>>>>> > >>>>>> Also, is there a reason why you haven't used ? > >>>>> > >>>>> So, in some ways devcoredump is ratelimited by userspace needing to > >>>>> clear an existing devcore.. > >>>>> > >>>> > >>>> Yes, a new devcoredump device will not be created until the previous one > >>>> is consumed or times out but here I am trying to limit even the DPU > >>>> snapshot capture because DPU register space is really huge and the rate > >>>> at which smmu faults occur is quite fast that its causing instability > >>>> while snapshots are being captured. > >>>> > >>>>> What I'd suggest would be more useful is to limit the devcores to once > >>>>> per atomic update, ie. if display state hasn't changed, maybe an > >>>>> additional devcore isn't useful > >>>>> > >>>>> BR, > >>>>> -R > >>>>> > >>>> > >>>> By display state change, do you mean like the checks we have in > >>>> drm_atomic_crtc_needs_modeset()? > >>>> > >>>> OR do you mean we need to cache the previous (currently picked up by hw) > >>>> state and trigger a new devcores if the new state is different by > >>>> comparing more things? > >>>> > >>>> This will help to reduce the snapshots to unique frame updates but I do > >>>> not think it will reduce the rate enough for the case where DPU did not > >>>> recover from the previous fault. > >>> > >>> I was thinking the easy thing, of just resetting the counter in > >>> msm_atomic_commit_tail().. I suppose we could be clever filter out > >>> updates that only change scanout address. Or hash the atomic state > >>> and only generate devcoredumps for unique states. But I'm not sure > >>> how over-complicated we should make this. > >>> > >>> BR, > >>> -R > >> > >> Its a good idea actually and I would also like to keep it simple :) > >> > >> One question, is it okay to assume that all compositors will only issue > >> unique commits? Because we are assuming thats the case with resetting > >> the counter in msm_atomic_commit_tail(). > > > > The compositors use drm_mode_atomic_ioctl() which allocates a new > > state each time. It is impossible to re-submit the same > > drm_atomic_state from userspace. > > > > No, what I meant was, is it okay to assume that a commit is issued only > when display configuration has changed? No. > Like if we get multiple commits for the same frame for some reason, > thats also spam and this approach will not avoid that. I'd say, new commit means that userspace thinks that something changed. So if the driver got another hang / issue / etc, it should try capturing a new state. -- With best wishes Dmitry
Re: [PATCH 5/5] drm/msm/dpu: rate limit snapshot capture for mmu faults
On Wed, 17 Jul 2024 at 01:43, Abhinav Kumar wrote: > > > > On 7/16/2024 2:50 PM, Rob Clark wrote: > > On Tue, Jul 16, 2024 at 2:45 PM Abhinav Kumar > > wrote: > >> > >> > >> > >> On 7/15/2024 12:51 PM, Rob Clark wrote: > >>> On Mon, Jul 1, 2024 at 12:43 PM Dmitry Baryshkov > >>> wrote: > >>>> > >>>> On Fri, Jun 28, 2024 at 02:48:47PM GMT, Abhinav Kumar wrote: > >>>>> There is no recovery mechanism in place yet to recover from mmu > >>>>> faults for DPU. We can only prevent the faults by making sure there > >>>>> is no misconfiguration. > >>>>> > >>>>> Rate-limit the snapshot capture for mmu faults to once per > >>>>> msm_kms_init_aspace() as that should be sufficient to capture > >>>>> the snapshot for debugging otherwise there will be a lot of > >>>>> dpu snapshots getting captured for the same fault which is > >>>>> redundant and also might affect capturing even one snapshot > >>>>> accurately. > >>>> > >>>> Please squash this into the first patch. There is no need to add code > >>>> with a known defficiency. > >>>> > >>>> Also, is there a reason why you haven't used ? > >>> > >>> So, in some ways devcoredump is ratelimited by userspace needing to > >>> clear an existing devcore.. > >>> > >> > >> Yes, a new devcoredump device will not be created until the previous one > >> is consumed or times out but here I am trying to limit even the DPU > >> snapshot capture because DPU register space is really huge and the rate > >> at which smmu faults occur is quite fast that its causing instability > >> while snapshots are being captured. > >> > >>> What I'd suggest would be more useful is to limit the devcores to once > >>> per atomic update, ie. if display state hasn't changed, maybe an > >>> additional devcore isn't useful > >>> > >>> BR, > >>> -R > >>> > >> > >> By display state change, do you mean like the checks we have in > >> drm_atomic_crtc_needs_modeset()? > >> > >> OR do you mean we need to cache the previous (currently picked up by hw) > >> state and trigger a new devcores if the new state is different by > >> comparing more things? > >> > >> This will help to reduce the snapshots to unique frame updates but I do > >> not think it will reduce the rate enough for the case where DPU did not > >> recover from the previous fault. > > > > I was thinking the easy thing, of just resetting the counter in > > msm_atomic_commit_tail().. I suppose we could be clever filter out > > updates that only change scanout address. Or hash the atomic state > > and only generate devcoredumps for unique states. But I'm not sure > > how over-complicated we should make this. > > > > BR, > > -R > > Its a good idea actually and I would also like to keep it simple :) > > One question, is it okay to assume that all compositors will only issue > unique commits? Because we are assuming thats the case with resetting > the counter in msm_atomic_commit_tail(). The compositors use drm_mode_atomic_ioctl() which allocates a new state each time. It is impossible to re-submit the same drm_atomic_state from userspace. -- With best wishes Dmitry
Re: [PATCH v5 03/16] drm/msm/dpu: move CRTC resource assignment to dpu_encoder_virt_atomic_mode_set
On Wed, 17 Jul 2024 at 01:40, Abhinav Kumar wrote:
>
>
>
> On 6/24/2024 2:13 PM, Dmitry Baryshkov wrote:
> > Historically CRTC resources (LMs and CTLs) were assigned in
> > dpu_crtc_atomic_begin(). The commit 9222cdd27e82 ("drm/msm/dpu: move hw
> > resource tracking to crtc state") simply moved resources to
> > struct dpu_crtc_state, without changing the code sequence. Later on the
> > commit b107603b4ad0 ("drm/msm/dpu: map mixer/ctl hw blocks in encoder
> > modeset") rearanged the code, but still kept the cstate->num_mixers
> > assignment to happen during commit phase. This makes dpu_crtc_state
> > inconsistent between consequent atomic_check() calls.
> >
> > Move CRTC resource assignment to happen at the end of
> > dpu_encoder_virt_atomic_check().
> >
> > Fixes: b107603b4ad0 ("drm/msm/dpu: map mixer/ctl hw blocks in encoder
> > modeset")
> > Signed-off-by: Dmitry Baryshkov
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 3 --
> > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 60
> > +++--
> > 2 files changed, 39 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > index 9f2164782844..7399794d75eb 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > @@ -1094,9 +1094,6 @@ static void dpu_crtc_disable(struct drm_crtc *crtc,
> > drm_for_each_encoder_mask(encoder, crtc->dev,
> > crtc->state->encoder_mask)
> > dpu_encoder_register_frame_event_callback(encoder, NULL,
> > NULL);
> >
> > - memset(cstate->mixers, 0, sizeof(cstate->mixers));
> > - cstate->num_mixers = 0;
> > -
>
> Any reason this part was removed?
>
> I think we still need this part.
>
> In dpu_encoder_get_topology(), we do not assign topology.num_lm based on
> state parameters, its based on catalog and intf_count which are decided
> at init time itself.
>
> Which means cstate->num_mixers will remain at a non-zero value even if
> we have NO_MODE which is what happens when we are disabling the CRTC
> during suspend or hotplug.
In the disable path the driver calls dpu_rm_release(), releasing all
resources. So there will be no assigned mixers, which matches num_lm =
0 in dpu_encoder_assign_crtc_resources().
>
> > /* disable clk & bw control until clk & bw properties are set */
> > cstate->bw_control = false;
> > cstate->bw_split_vote = false;
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > index 7613005fbfea..98f3a8d84300 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > @@ -628,6 +628,41 @@ static struct msm_display_topology
> > dpu_encoder_get_topology(
> > return topology;
> > }
> >
> > +static void dpu_encoder_assign_crtc_resources(struct dpu_kms *dpu_kms,
> > + struct drm_encoder *drm_enc,
> > + struct dpu_global_state
> > *global_state,
> > + struct drm_crtc_state
> > *crtc_state)
> > +{
> > + struct dpu_crtc_state *cstate;
> > + struct dpu_hw_blk *hw_ctl[MAX_CHANNELS_PER_ENC];
> > + struct dpu_hw_blk *hw_lm[MAX_CHANNELS_PER_ENC];
> > + struct dpu_hw_blk *hw_dspp[MAX_CHANNELS_PER_ENC];
> > + int num_lm, num_ctl, num_dspp, i;
> > +
> > + cstate = to_dpu_crtc_state(crtc_state);
> > +
> > + memset(cstate->mixers, 0, sizeof(cstate->mixers));
> > +
> > + num_ctl = dpu_rm_get_assigned_resources(_kms->rm, global_state,
> > + drm_enc->base.id, DPU_HW_BLK_CTL, hw_ctl, ARRAY_SIZE(hw_ctl));
> > + num_lm = dpu_rm_get_assigned_resources(_kms->rm, global_state,
> > + drm_enc->base.id, DPU_HW_BLK_LM, hw_lm, ARRAY_SIZE(hw_lm));
> > + num_dspp = dpu_rm_get_assigned_resources(_kms->rm, global_state,
> > + drm_enc->base.id, DPU_HW_BLK_DSPP, hw_dspp,
> > + ARRAY_SIZE(hw_dspp));
> > +
> > + for (i = 0; i < num_lm; i++) {
> > + int ctl_idx = (i < num_ctl) ? i : (num_ctl-1);
> > +
> > + cstate->mixers[i].hw_lm = to_dpu_hw_mixer(hw_lm[i]);
> > + cstate->mixers[i].lm_ctl = to_dpu_hw_ctl(hw_ctl[ctl_idx]);
> > +
Re: DisplayPort: handling of HPD events / link training
On Tue, Jul 16, 2024 at 06:48:12PM GMT, Thomas Zimmermann wrote:
> Hi
>
> Am 16.07.24 um 18:35 schrieb Dmitry Baryshkov:
> > On Tue, 16 Jul 2024 at 18:58, Thomas Zimmermann wrote:
> > > Hi
> > >
> > > Am 27.02.24 um 23:40 schrieb Dmitry Baryshkov:
> > > > Hello,
> > > >
> > > > We are currently looking at checking and/or possibly redesigning the
> > > > way the MSM DRM driver handles the HPD events and link training.
> > > >
> > > > After a quick glance at the drivers implementing DP support, I noticed
> > > > following main approaches:
> > > > - Perform link training at the atomic_enable time, don't report
> > > > failures (mtk, analogix, zynqmp, tegra, nouveau)
> > > > - Perform link training at the atomic_enable time, report errors using
> > > > link_status property (i915, mhdp8546)
> > > > - Perform link training on the plug event (msm, it8605).
> > > > - Perform link training from the DPMS handler, also calling it from
> > > > the enable callback (AMDGPU, radeon).
> > > >
> > > > It looks like the majority wins and we should move HPD to
> > > > atomic_enable time. Is that assumption correct?
> > > Did you ever receive an answer to this question? I currently investigate
> > > ast's DP code, which does link training as part of detecting the
> > > connector state (in detect_ctx). But most other drivers do this in
> > > atomic_enable. I wonder if ast should follow.
> > Short answer: yes, the only proper place to do it is atomic_enable().
>
> Thanks.
>
> >
> > Long answer: I don't see a way to retrigger link training in ast_dp.c
> > Without such change you are just shifting things around. The
> > end-result of moving link-training to atomic_enable() is that each
> > enable can trigger link training, possibly lowering the link rate,
> > etc. if link training is just a status bit from the firmware that we
> > don't control, it doesn't make real-real sense to move it.
>
> I have to think about what to do. People tend to copy existing drivers,
> which alone might be a good argument for using atomic_enable. The link
> training is indeed just a flag that is set by the firmware. I think it's
> possible to re-trigger training by powering the port down and up again.
> atomic_enable could likely do that. The hardware is also somewhat buggy and
> not fully standard conformant.
It stil looks like having an explicit comment ('check LT here becasue
handled by firmware') might be better.
--
With best wishes
Dmitry
Re: [PATCH] drm/msm/adreno: Fix error return if missing firmware-name
On Tue, Jul 16, 2024 at 09:06:30AM GMT, Rob Clark wrote: > From: Rob Clark > > -ENODEV is used to signify that there is no zap shader for the platform, > and the CPU can directly take the GPU out of secure mode. We want to > use this return code when there is no zap-shader node. But not when > there is, but without a firmware-name property. This case we want to > treat as-if the needed fw is not found. > > Signed-off-by: Rob Clark > --- > drivers/gpu/drm/msm/adreno/adreno_gpu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH v5 4/4] arm64: dts: qcom: add HDMI nodes for msm8998
On Tue, 16 Jul 2024 at 17:34, Marc Gonzalez wrote:
>
> On 16/07/2024 15:11, Konrad Dybcio wrote:
>
> > On 27.06.2024 5:54 PM, Marc Gonzalez wrote:
> >
> >> arch/arm64/boot/dts/qcom/msm8998.dtsi | 100
> >> +-
> >> 1 file changed, 99 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi
> >> b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> >> index ba5e873f0f35f..417c12534823f 100644
> >> --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
> >> +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
> >> @@ -2785,7 +2785,7 @@ mmcc: clock-controller@c8c {
> >> <_dsi0_phy 0>,
> >> <_dsi1_phy 1>,
> >> <_dsi1_phy 0>,
> >> - <0>,
> >> + <_phy 0>,
> >> <0>,
> >> <0>,
> >> < GCC_MMSS_GPLL0_DIV_CLK>;
> >> @@ -2890,6 +2890,14 @@ dpu_intf2_out: endpoint {
> >> remote-endpoint =
> >> <_dsi1_in>;
> >> };
> >> };
> >> +
> >> +port@2 {
> >> +reg = <2>;
> >> +
> >> +dpu_intf3_out: endpoint {
> >> +remote-endpoint =
> >> <_in>;
> >> +};
> >> +};
> >> };
> >> };
> >>
> >> @@ -3045,6 +3053,96 @@ mdss_dsi1_phy: phy@c996400 {
> >>
> >> status = "disabled";
> >> };
> >> +
> >> +hdmi: hdmi-tx@c9a {
> >
> > Please prefix the labels (hdmi: and hdmi_phy:) with mdss_
> >
> > Otherwise, this looks good
> >
> > Reviewed-by: Konrad Dybcio
> >
> >
> > One thing I noticed (testing on the 8998 MTP), enabling MDSS (not
> > necessarily
> > HDMI, mdss and mdp is enough) results in SMMU lockups about 30% of the
> > time..
> >
> > [4.911422] arm-smmu cd0.iommu: FSR= 0402 [Format=2 TF],
> > SID=0x0
> > [4.913412] platform c901000.display-controller: Fixed dependency
> > cycle(s) with /soc@0/display-subsystem@c90/hdmi-tx@c9a
> > [4.923353] arm-smmu cd0.iommu: FSYNR0 = 0021 [S1CBNDX=0 PNU
> > PLVL=1]
> > [4.927893] arm-smmu cd0.iommu: FSR= 0402 [Format=2 TF],
> > SID=0x0
> > [4.930647] platform c9a.hdmi-tx: Fixed dependency cycle(s) with
> > /soc@0/display-subsystem@c90/display-controller@c901000
> > [4.941928] arm-smmu cd0.iommu: FSYNR0 = 0021 [S1CBNDX=0 PNU
> > PLVL=1]
> > [4.944438] arm-smmu cd0.iommu: FSR= 0402 [Format=2 TF],
> > SID=0x0
> > [4.952338] msm_hdmi_phy c9a0600.hdmi-phy: supply vddio not found, using
> > dummy regulator
> > [4.956013] arm-smmu cd0.iommu: FSYNR0 = 0021 [S1CBNDX=0 PNU
> > PLVL=1]
> > [4.961055] arm-smmu cd0.iommu: FSR= 0402 [Format=2 TF],
> > SID=0x0
> > [4.967917] msm_hdmi_phy c9a0600.hdmi-phy: supply vcca not found, using
> > dummy regulator
> > [4.974565] arm-smmu cd0.iommu: FSYNR0 = 0021 [S1CBNDX=0 PNU
> > PLVL=1]
> > [4.977628] arm-smmu cd0.iommu: FSR= 0402 [Format=2 TF],
> > SID=0x0
> > [4.984122] Bluetooth: hci0: setting up wcn399x
> > [4.989670] arm-smmu cd0.iommu: FSYNR0 = 0021 [S1CBNDX=0 PNU
> > PLVL=1]
>
> Interesting. I don't think I've noticed any lock-ups
> across multiple reboots.
>
> FWIW, I get similar warnings about "Fixed dependency cycle(s)" on my custom
> DT.
>
> [0.055349] platform 1da4000.ufshc: Fixed dependency cycle(s) with
> /soc@0/phy@1da7000
> [0.055525] platform 1da4000.ufshc: Fixed dependency cycle(s) with
> /soc@0/phy@1da7000
> [0.055584] platform 1da7000.phy: Fixed dependency cycle(s) with
> /soc@0/ufshc@1da4000
> [0.060279] platform c8c.clock-controller: Fixed dependency cycle(s)
> with /soc@0/display-subsystem@c90/hdmi-phy@c9a0600
> [0.060494] platform c90.display-subsystem: Fixed dependency cycle(s)
> with /soc@0/clock-controller@c8c
> [0.062432] platform hdmi-out: Fixed dependency cycle(s) with
> /soc@0/i2c@c1b5000/tdp158@5e
> ...
> [ 18.534346] adreno 500.gpu: Adding to iommu group 2
> [ 18.540215] msm-mdss c90.display-subsystem: Adding to iommu group 3
> [ 18.544695] platform c901000.display-controller: Fixed dependency cycle(s)
> with /soc@0/display-subsystem@c90/hdmi-tx@c9a
> [ 18.551239] platform c901000.display-controller: Fixed dependency cycle(s)
> with /soc@0/display-subsystem@c90/hdmi-tx@c9a
> [ 18.562685] platform c9a.hdmi-tx: Fixed dependency cycle(s) with
> /soc@0/i2c@c1b5000/tdp158@5e
> [ 18.574122]
Re: DisplayPort: handling of HPD events / link training
On Tue, 16 Jul 2024 at 18:58, Thomas Zimmermann wrote: > > Hi > > Am 27.02.24 um 23:40 schrieb Dmitry Baryshkov: > > Hello, > > > > We are currently looking at checking and/or possibly redesigning the > > way the MSM DRM driver handles the HPD events and link training. > > > > After a quick glance at the drivers implementing DP support, I noticed > > following main approaches: > > - Perform link training at the atomic_enable time, don't report > > failures (mtk, analogix, zynqmp, tegra, nouveau) > > - Perform link training at the atomic_enable time, report errors using > > link_status property (i915, mhdp8546) > > - Perform link training on the plug event (msm, it8605). > > - Perform link training from the DPMS handler, also calling it from > > the enable callback (AMDGPU, radeon). > > > > It looks like the majority wins and we should move HPD to > > atomic_enable time. Is that assumption correct? > > Did you ever receive an answer to this question? I currently investigate > ast's DP code, which does link training as part of detecting the > connector state (in detect_ctx). But most other drivers do this in > atomic_enable. I wonder if ast should follow. Short answer: yes, the only proper place to do it is atomic_enable(). Long answer: I don't see a way to retrigger link training in ast_dp.c Without such change you are just shifting things around. The end-result of moving link-training to atomic_enable() is that each enable can trigger link training, possibly lowering the link rate, etc. if link training is just a status bit from the firmware that we don't control, it doesn't make real-real sense to move it. > > Best regards > Thomas > > > > > Also two related questions: > > - Is there a plan to actually make use of the link_status property? > > Intel presented it at FOSDEM 2018, but since that time it was not > > picked up by other drivers. > > > > - Is there any plan to create generic DP link training helpers? After > > glancing through the DP drivers there is a lot of similar code in the > > link training functions, with minor differences here and there. And > > it's those minor differences that bug me. It means that drivers might > > respond differently to similar devices. Or that there might be minor > > bugs here and there. > > > > -- > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Frankenstrasse 146, 90461 Nuernberg, Germany > GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman > HRB 36809 (AG Nuernberg) > -- With best wishes Dmitry
Re: [PATCH v6 3/6] drm/msm/hdmi: add "qcom,hdmi-tx-8998" compatible
On Mon, 15 Jul 2024 at 18:28, Conor Dooley wrote:
>
> On Mon, Jul 15, 2024 at 04:26:12PM +0100, Conor Dooley wrote:
> > On Mon, Jul 15, 2024 at 02:21:16PM +0200, Marc Gonzalez wrote:
> > > Current driver already supports the msm8998 HDMI TX.
> > > We just need to add the compatible string.
> >
> > Why is this required when the driver change suggests that this device is
> > compatible with the existing 8974?
>
> (I know I reviewed the binding already, just noticing this which
> suggests a fallback would be appropriate, despite the differing clocks
> etc)
Yes and no. All supported MMS HDMI controllers are backwards
compatible at least back to msm8960. However as we were not using
fallbacks before, does it really make sense to introduce them now?
It's highly likely that there will be no new HDMI controllers
(Qualcomm has stopped using them with MSM8998 at 2018).
>
> >
> > >
> > > Signed-off-by: Marc Gonzalez
> > > ---
> > > drivers/gpu/drm/msm/hdmi/hdmi.c | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c
> > > b/drivers/gpu/drm/msm/hdmi/hdmi.c
> > > index 24abcb7254cc4..0bfee41c2e71a 100644
> > > --- a/drivers/gpu/drm/msm/hdmi/hdmi.c
> > > +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
> > > @@ -549,6 +549,7 @@ static void msm_hdmi_dev_remove(struct
> > > platform_device *pdev)
> > > }
> > >
> > > static const struct of_device_id msm_hdmi_dt_match[] = {
> > > + { .compatible = "qcom,hdmi-tx-8998", .data = _tx_8974_config },
> > > { .compatible = "qcom,hdmi-tx-8996", .data = _tx_8974_config },
> > > { .compatible = "qcom,hdmi-tx-8994", .data = _tx_8974_config },
> > > { .compatible = "qcom,hdmi-tx-8084", .data = _tx_8974_config },
> > >
> > > --
> > > 2.34.1
> > >
>
>
--
With best wishes
Dmitry
Re: [PATCH v6 4/6] drm/msm: add msm8998 hdmi phy/pll support
On Mon, Jul 15, 2024 at 02:21:17PM GMT, Marc Gonzalez wrote: > From: Arnaud Vrac > > Add support for the HDMI PHY as present on the Qualcomm MSM8998 SoC. > This code is mostly copy & paste of the vendor code from msm-4.4 > kernel.lnx.4.4.r38-rel. > > Signed-off-by: Arnaud Vrac > Signed-off-by: Marc Gonzalez > --- > drivers/gpu/drm/msm/Makefile | 1 + > drivers/gpu/drm/msm/hdmi/hdmi.h| 8 + > drivers/gpu/drm/msm/hdmi/hdmi_phy.c| 5 + > drivers/gpu/drm/msm/hdmi/hdmi_phy_8998.c | 779 > + > drivers/gpu/drm/msm/registers/display/hdmi.xml | 89 +++ > 5 files changed, 882 insertions(+) > Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH v6 3/6] drm/msm/hdmi: add "qcom,hdmi-tx-8998" compatible
On Mon, Jul 15, 2024 at 02:21:16PM GMT, Marc Gonzalez wrote: > Current driver already supports the msm8998 HDMI TX. > We just need to add the compatible string. > > Signed-off-by: Marc Gonzalez > --- > drivers/gpu/drm/msm/hdmi/hdmi.c | 1 + > 1 file changed, 1 insertion(+) > Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
[PATCH v4 5/5] drm/bridge-connector: reset the HDMI connector state
On HDMI connectors which use drm_bridge_connector and DRM_BRIDGE_OP_HDMI
IGT chokes on the max_bpc property in several kms_properties tests due
to the drm_bridge_connector failing to reset HDMI-related
properties.
Call __drm_atomic_helper_connector_hdmi_reset() if the
drm_bridge_connector has bridge_hdmi.
It is impossible to call this function from HDMI bridges, none of the
bridge callbacks correspond to the drm_connector_funcs::reset().
Fixes: 6b4468b0c6ba ("drm/bridge-connector: implement glue code for HDMI
connector")
Reviewed-by: Maxime Ripard
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/display/Kconfig| 1 +
drivers/gpu/drm/display/drm_bridge_connector.c | 13 -
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig
index 8c174ceb0c4d..3763649ba251 100644
--- a/drivers/gpu/drm/display/Kconfig
+++ b/drivers/gpu/drm/display/Kconfig
@@ -15,6 +15,7 @@ if DRM_DISPLAY_HELPER
config DRM_BRIDGE_CONNECTOR
bool
+ select DRM_DISPLAY_HDMI_STATE_HELPER
help
DRM connector implementation terminating DRM bridge chains.
diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c
b/drivers/gpu/drm/display/drm_bridge_connector.c
index 0869b663f17e..7ebb35438459 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -216,8 +216,19 @@ static void drm_bridge_connector_debugfs_init(struct
drm_connector *connector,
}
}
+static void drm_bridge_connector_reset(struct drm_connector *connector)
+{
+ struct drm_bridge_connector *bridge_connector =
+ to_drm_bridge_connector(connector);
+
+ drm_atomic_helper_connector_reset(connector);
+ if (bridge_connector->bridge_hdmi)
+ __drm_atomic_helper_connector_hdmi_reset(connector,
+connector->state);
+}
+
static const struct drm_connector_funcs drm_bridge_connector_funcs = {
- .reset = drm_atomic_helper_connector_reset,
+ .reset = drm_bridge_connector_reset,
.detect = drm_bridge_connector_detect,
.fill_modes = drm_helper_probe_single_connector_modes,
.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
--
2.39.2
[PATCH v4 3/5] drm/connector: automatically set immutable flag for max_bpc property
With the introduction of the HDMI Connector framework the driver might
end up creating the max_bpc property with min = max = 8. IGT insists
that such properties carry the 'immutable' flag. Automatically set the
flag if the driver asks for the max_bpc property with min == max.
Fixes: aadb3e16b8f3 ("drm/connector: hdmi: Add output BPC to the connector
state")
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/drm_connector.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index ab6ab7ff7ea8..33847fd63628 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2610,7 +2610,12 @@ int drm_connector_attach_max_bpc_property(struct
drm_connector *connector,
prop = connector->max_bpc_property;
if (!prop) {
- prop = drm_property_create_range(dev, 0, "max bpc", min, max);
+ u32 flags = 0;
+
+ if (min == max)
+ flags |= DRM_MODE_PROP_IMMUTABLE;
+
+ prop = drm_property_create_range(dev, flags, "max bpc", min,
max);
if (!prop)
return -ENOMEM;
--
2.39.2
[PATCH v4 4/5] drm/bridge-connector: move to DRM_DISPLAY_HELPER module
drm_bridge_connector is a "leaf" driver, belonging to the display
helper, rather than the "CRTC" drm_kms_helper module. Move the driver
to the drm/display and add necessary Kconfig selection clauses.
Suggested-by: Maxime Ripard
Signed-off-by: Dmitry Baryshkov
---
MAINTAINERS | 2 +-
drivers/gpu/drm/Makefile | 1 -
drivers/gpu/drm/bridge/Kconfig | 1 +
drivers/gpu/drm/display/Kconfig | 5 +
drivers/gpu/drm/display/Makefile | 2 ++
drivers/gpu/drm/{ => display}/drm_bridge_connector.c | 0
drivers/gpu/drm/imx/dcss/Kconfig | 2 ++
drivers/gpu/drm/imx/lcdc/Kconfig | 2 ++
drivers/gpu/drm/ingenic/Kconfig | 2 ++
drivers/gpu/drm/kmb/Kconfig | 2 ++
drivers/gpu/drm/mediatek/Kconfig | 2 ++
drivers/gpu/drm/meson/Kconfig| 2 ++
drivers/gpu/drm/msm/Kconfig | 1 +
drivers/gpu/drm/omapdrm/Kconfig | 2 ++
drivers/gpu/drm/renesas/rcar-du/Kconfig | 2 ++
drivers/gpu/drm/renesas/rz-du/Kconfig| 2 ++
drivers/gpu/drm/renesas/shmobile/Kconfig | 2 ++
drivers/gpu/drm/rockchip/Kconfig | 4
drivers/gpu/drm/tegra/Kconfig| 1 +
drivers/gpu/drm/tidss/Kconfig| 2 ++
drivers/gpu/drm/xlnx/Kconfig | 1 +
21 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index fb1df8c29f5a..5b5520ed57fe 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7463,8 +7463,8 @@ S:Maintained
T: git https://gitlab.freedesktop.org/drm/misc/kernel.git
F: Documentation/devicetree/bindings/display/bridge/
F: drivers/gpu/drm/bridge/
+F: drivers/gpu/drm/display/drm_bridge_connector.c
F: drivers/gpu/drm/drm_bridge.c
-F: drivers/gpu/drm/drm_bridge_connector.c
F: include/drm/drm_bridge.h
F: include/drm/drm_bridge_connector.h
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 68cc9258ffc4..fa432a1ac9e2 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -128,7 +128,6 @@ obj-$(CONFIG_DRM_TTM_HELPER) += drm_ttm_helper.o
drm_kms_helper-y := \
drm_atomic_helper.o \
drm_atomic_state_helper.o \
- drm_bridge_connector.o \
drm_crtc_helper.o \
drm_damage_helper.o \
drm_encoder_slave.o \
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index c621be1a99a8..3eb955333c80 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -390,6 +390,7 @@ config DRM_TI_SN65DSI86
depends on OF
select DRM_DISPLAY_DP_HELPER
select DRM_DISPLAY_HELPER
+ select DRM_BRIDGE_CONNECTOR
select DRM_KMS_HELPER
select REGMAP_I2C
select DRM_PANEL
diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig
index 9c2da1e48b75..8c174ceb0c4d 100644
--- a/drivers/gpu/drm/display/Kconfig
+++ b/drivers/gpu/drm/display/Kconfig
@@ -13,6 +13,11 @@ config DRM_DISPLAY_HELPER
if DRM_DISPLAY_HELPER
+config DRM_BRIDGE_CONNECTOR
+ bool
+ help
+ DRM connector implementation terminating DRM bridge chains.
+
config DRM_DISPLAY_DP_AUX_CEC
bool "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support"
select DRM_DISPLAY_DP_HELPER
diff --git a/drivers/gpu/drm/display/Makefile b/drivers/gpu/drm/display/Makefile
index a023f72fa139..629c834c3192 100644
--- a/drivers/gpu/drm/display/Makefile
+++ b/drivers/gpu/drm/display/Makefile
@@ -3,6 +3,8 @@
obj-$(CONFIG_DRM_DISPLAY_DP_AUX_BUS) += drm_dp_aux_bus.o
drm_display_helper-y := drm_display_helper_mod.o
+drm_display_helper-$(CONFIG_DRM_BRIDGE_CONNECTOR) += \
+ drm_bridge_connector.o
drm_display_helper-$(CONFIG_DRM_DISPLAY_DP_HELPER) += \
drm_dp_dual_mode_helper.o \
drm_dp_helper.o \
diff --git a/drivers/gpu/drm/drm_bridge_connector.c
b/drivers/gpu/drm/display/drm_bridge_connector.c
similarity index 100%
rename from drivers/gpu/drm/drm_bridge_connector.c
rename to drivers/gpu/drm/display/drm_bridge_connector.c
diff --git a/drivers/gpu/drm/imx/dcss/Kconfig b/drivers/gpu/drm/imx/dcss/Kconfig
index 3ffc061d392b..59e3b6a1dff0 100644
--- a/drivers/gpu/drm/imx/dcss/Kconfig
+++ b/drivers/gpu/drm/imx/dcss/Kconfig
@@ -2,6 +2,8 @@ config DRM_IMX_DCSS
tristate "i.MX8MQ DCSS"
select IMX_IRQSTEER
select DRM_KMS_HELPER
+ select DRM_DISPLAY_HELPER
+ select DRM_BRIDGE_CONNECTOR
select DRM_GEM_DMA_HELPER
select VIDEOMODE_HELPERS
depends on DRM && ARCH_MXC && ARM64
diff --git a/drivers/gpu/drm/imx/lcdc/Kconfig b/drivers/gpu/drm/imx/lcdc/Kconfig
index 7e57922bbd9d..9c28bb0f4662 1006
[PATCH v4 2/5] drm/drm_property: require DRM_MODE_PROP_IMMUTABLE for single-value props
Document that DRM_MODE_PROP_IMMUTABLE must be set for the properties
that are immutable by definition - e.g. ranges with min == max or enums
with a single value. This matches the behaviour of the IGT tests, see
kms_properties.c / validate_range_prop(), validate_enum_prop(),
validate_bitmask_prop().
Signed-off-by: Dmitry Baryshkov
---
include/drm/drm_property.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/drm/drm_property.h b/include/drm/drm_property.h
index 082f29156b3e..d78ec42de42f 100644
--- a/include/drm/drm_property.h
+++ b/include/drm/drm_property.h
@@ -162,6 +162,9 @@ struct drm_property {
* userspace, e.g. the EDID, or the connector path property on DP
* MST sinks. Kernel can update the value of an immutable property
* by calling drm_object_property_set_value().
+* This flag MUST be set for all properties that have only a
+* single value (e.g. min == max or if enum has only a single
+* value).
*/
uint32_t flags;
--
2.39.2
[PATCH v4 1/5] drm/display: stop depending on DRM_DISPLAY_HELPER
Kconfig symbols should not declare dependency on DRM_DISPLAY_HELPER. Move all parts of DRM_DISPLAY_HELPER to an if DRM_DISPLAY_HELPER block. It is not possible to make those symbols select DRM_DISPLAY_HELPER because of the link issues when a part of the helper is selected to be built-in, while other part is selected to be as module. In such a case the modular part doesn't get built at all, leading to undefined symbols. The only viable alternative is to split drm_display_helper.ko into several small modules, each of them having their own dependencies. Suggested-by: Maxime Ripard Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/display/Kconfig | 19 --- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig index a2e42014ffe0..9c2da1e48b75 100644 --- a/drivers/gpu/drm/display/Kconfig +++ b/drivers/gpu/drm/display/Kconfig @@ -1,19 +1,20 @@ # SPDX-License-Identifier: MIT +config DRM_DISPLAY_DP_AUX_BUS + tristate + depends on DRM + depends on OF || COMPILE_TEST + config DRM_DISPLAY_HELPER tristate depends on DRM help DRM helpers for display adapters. -config DRM_DISPLAY_DP_AUX_BUS - tristate - depends on DRM - depends on OF || COMPILE_TEST +if DRM_DISPLAY_HELPER config DRM_DISPLAY_DP_AUX_CEC bool "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support" - depends on DRM && DRM_DISPLAY_HELPER select DRM_DISPLAY_DP_HELPER select CEC_CORE help @@ -25,7 +26,6 @@ config DRM_DISPLAY_DP_AUX_CEC config DRM_DISPLAY_DP_AUX_CHARDEV bool "DRM DP AUX Interface" - depends on DRM && DRM_DISPLAY_HELPER select DRM_DISPLAY_DP_HELPER help Choose this option to enable a /dev/drm_dp_auxN node that allows to @@ -34,7 +34,6 @@ config DRM_DISPLAY_DP_AUX_CHARDEV config DRM_DISPLAY_DP_HELPER bool - depends on DRM_DISPLAY_HELPER help DRM display helpers for DisplayPort. @@ -61,25 +60,23 @@ config DRM_DISPLAY_DP_TUNNEL_STATE_DEBUG config DRM_DISPLAY_DSC_HELPER bool - depends on DRM_DISPLAY_HELPER help DRM display helpers for VESA DSC (used by DSI and DisplayPort). config DRM_DISPLAY_HDCP_HELPER bool - depends on DRM_DISPLAY_HELPER help DRM display helpers for HDCP. config DRM_DISPLAY_HDMI_HELPER bool - depends on DRM_DISPLAY_HELPER help DRM display helpers for HDMI. config DRM_DISPLAY_HDMI_STATE_HELPER bool - depends on DRM_DISPLAY_HELPER select DRM_DISPLAY_HDMI_HELPER help DRM KMS state helpers for HDMI. + +endif # DRM_DISPLAY_HELPER -- 2.39.2
[PATCH v4 0/5] drm: fix two issues related to HDMI Connector implementation
Running IGT tests on Qualcomm Dragonboard820c uncovered two issues with
the HDMI Connector implementation and with its integration into the
drm_bridge_connector. Fix those issues.
Note, I'm not fully satisfied with the drm_bridge_connector move. Maybe
it's better to add drm_bridge_funcs::connector_reset() and call it from
__drm_atomic_helper_connector_reset().
Depends on
https://lore.kernel.org/dri-devel/20240704-panel-sw43408-fix-v6-1-3ea1c94bb...@linaro.org
Signed-off-by: Dmitry Baryshkov
---
Changes in v4:
- Fixed DRM_MODE_PROP_IMMUTABLE to use MUST in the single-value clause (Maxime)
- Rebased on top of DRM_DSC_HELPERS patch
- Removed 'depends on DRM_DISPLAY_HELPER' (Maxime)
- Link to v3:
https://lore.kernel.org/r/20240702-drm-bridge-connector-fix-hdmi-reset-v3-0-12b0e3124...@linaro.org
Changes in v3:
- Document the DRM_MODE_PROP_IMMUTABLE requirements currently exposed
only via IGT tests (Maxime).
- Move drm_bridge_connector to drm_display_helper.
- Link to v2:
https://lore.kernel.org/r/20240623-drm-bridge-connector-fix-hdmi-reset-v2-0-8590d4491...@linaro.org
Changes in v2:
- Actually pass the flags to drm_property_create_range().
- Link to v1:
https://lore.kernel.org/r/20240623-drm-bridge-connector-fix-hdmi-reset-v1-0-41e9894dc...@linaro.org
---
Dmitry Baryshkov (5):
drm/display: stop depending on DRM_DISPLAY_HELPER
drm/drm_property: require DRM_MODE_PROP_IMMUTABLE for single-value props
drm/connector: automatically set immutable flag for max_bpc property
drm/bridge-connector: move to DRM_DISPLAY_HELPER module
drm/bridge-connector: reset the HDMI connector state
MAINTAINERS| 2 +-
drivers/gpu/drm/Makefile | 1 -
drivers/gpu/drm/bridge/Kconfig | 1 +
drivers/gpu/drm/display/Kconfig| 25 --
drivers/gpu/drm/display/Makefile | 2 ++
.../gpu/drm/{ => display}/drm_bridge_connector.c | 13 ++-
drivers/gpu/drm/drm_connector.c| 7 +-
drivers/gpu/drm/imx/dcss/Kconfig | 2 ++
drivers/gpu/drm/imx/lcdc/Kconfig | 2 ++
drivers/gpu/drm/ingenic/Kconfig| 2 ++
drivers/gpu/drm/kmb/Kconfig| 2 ++
drivers/gpu/drm/mediatek/Kconfig | 2 ++
drivers/gpu/drm/meson/Kconfig | 2 ++
drivers/gpu/drm/msm/Kconfig| 1 +
drivers/gpu/drm/omapdrm/Kconfig| 2 ++
drivers/gpu/drm/renesas/rcar-du/Kconfig| 2 ++
drivers/gpu/drm/renesas/rz-du/Kconfig | 2 ++
drivers/gpu/drm/renesas/shmobile/Kconfig | 2 ++
drivers/gpu/drm/rockchip/Kconfig | 4
drivers/gpu/drm/tegra/Kconfig | 1 +
drivers/gpu/drm/tidss/Kconfig | 2 ++
drivers/gpu/drm/xlnx/Kconfig | 1 +
include/drm/drm_property.h | 3 +++
23 files changed, 68 insertions(+), 15 deletions(-)
---
base-commit: cfbc154f11aaa32b4b2887323e4372390648046d
change-id: 20240623-drm-bridge-connector-fix-hdmi-reset-0ce86af053aa
Best regards,
--
Dmitry Baryshkov
Re: [PATCH] drm/msm/dp: enable widebus on all relevant chipsets
On Thu, Jul 11, 2024 at 03:48:50PM GMT, Abhinav Kumar wrote: > Hardware document indicates that widebus is recommended on DP on all > MDSS chipsets starting version 5.x.x and above. > > Follow the guideline and mark widebus support on all relevant > chipsets for DP. > > Signed-off-by: Abhinav Kumar > --- > drivers/gpu/drm/msm/dp/dp_display.c | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) > Although it doesn't seem to fix the 4k screen corruption, I think it's still a proper patch (and we should be following hardware documentation). With the Fixes tags in place: Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH 2/2] drm/msm/dpu: don't play tricks with debug macros
On Thu, Jul 11, 2024 at 11:03:15AM GMT, Abhinav Kumar wrote: > > > On 7/10/2024 12:40 AM, Dmitry Baryshkov wrote: > > On Tue, 9 Jul 2024 at 22:39, Abhinav Kumar > > wrote: > > > > > > > > > > > > On 7/9/2024 6:48 AM, Dmitry Baryshkov wrote: > > > > DPU debugging macros need to be converted to a proper drm_debug_* > > > > macros, however this is a going an intrusive patch, not suitable for a > > > > fix. Wire DPU_DEBUG and DPU_DEBUG_DRIVER to always use DRM_DEBUG_DRIVER > > > > to make sure that DPU debugging messages always end up in the drm debug > > > > messages and are controlled via the usual drm.debug mask. > > > > > > > > > > These macros have been deprecated, is this waht you meant by the > > > conversion to proper drm_debug_*? > > > > Yes. Drop the driver-specific wrappers where they don't make sense. > > Use sensible format strings in the cases where it actually does (like > > VIDENC or _PLANE) > > > > Ack but we need to not just drop the wrappers but drop the usage of these > macros as well because it is documented that they are deprecated. > > So I assume you want to get this in and do that as a follow up change? Yes, somewhere in the long list of cleanups. I have a similar item against DP driver, which uses correct macros, > > > /* NOTE: this is deprecated in favor of drm_dbg(NULL, ...). */ > > > #define DRM_DEBUG_DRIVER(fmt, ...) \ > > > __drm_dbg(DRM_UT_DRIVER, fmt, ##__VA_ARGS__) > > > > > > I think all that this macro was doing was to have appropriate DRM_UT_* > > > macros enabled before calling the corresponding DRM_DEBUG_* macros. But > > > I think what was incorrect here is for DPU_DEBUG, we could have used > > > DRM_UT_CORE instead of DRM_UT_KMS. > > > > It pretty much tries to overplay the existing drm debugging mechanism > > by either sending the messages to the DRM channel or just using > > pr_debug. With DYNAMIC_DEBUG being disabled pr_debug is just an empty > > macro, so all the messages can end up in /dev/null. We should not be > > trying to be too smart, using standard DRM_DEBUG_DRIVER should be > > enough. This way all driver-related messages are controlled by > > drm.debug including or excluding the 0x02 bit. > > > > > > > > > > And DRM_DEBUG_DRIVER should have been used instead of DRM_ERROR. > > > > > > Was this causing the issue of the prints not getting enabled? > > > > I pretty much think so. > > > > Alright, I am okay with the approach, just one minor suggestion, to keep the > behavior intact, previously the code wanted DPU_DEBUG to be controlled by > DRM_UT_KMS and DPU_DEBUG_DRIVER controlled by DRM_UT_DRIVER. > > Keeping that intact, we need to use DRM_DEBUG_KMS for DPU_DEBUG? I might make that more explicit: I don't think that it is a good idea for a generic DPU_DEBUG macro to be tied to DRM_UT_KMS. We are reporting a debug message from driver, so by default it should go to the DRM_UT_DRIVER channel. While refactoring things we might end up with messages going to ATOMIC or KMS, but DRIVER should be the default. -- With best wishes Dmitry
Re: [PATCH v5 02/16] drm/msm/dpu: fix error condition in dpu_encoder_virt_atomic_mode_set
On Sat, 13 Jul 2024 at 03:25, Abhinav Kumar wrote:
>
>
>
> On 7/12/2024 4:11 PM, Dmitry Baryshkov wrote:
> > On Fri, 12 Jul 2024 at 22:41, Abhinav Kumar
> > wrote:
> >> On 6/24/2024 2:13 PM, Dmitry Baryshkov wrote:
> >>> The commit b954fa6baaca ("drm/msm/dpu: Refactor rm iterator") removed
> >>> zero-init of the hw_ctl array, but didn't change the error condition,
> >>> that checked for hw_ctl[i] being NULL. Use indices check instead.
> >>>
> >>> Fixes: b954fa6baaca ("drm/msm/dpu: Refactor rm iterator")
> >>> Signed-off-by: Dmitry Baryshkov
> >>> ---
> >>>drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
> >>>1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >>> index 5d205e09cf45..7613005fbfea 100644
> >>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> >>> @@ -1186,7 +1186,7 @@ static void :tag(struct drm_encoder *drm_enc,
> >>>return;
> >>>}
> >>>
> >>> - if (!hw_ctl[i]) {
> >>> + if (i >= num_ctl) {
> >>
> >> This is not very clear to me.
> >>
> >> How will we hit this condition? I dont see i going beyond 1 in this loop
> >> and neither should num_ctl
> >
> > Why? the driver doesn't support flushing through a single CTL, so
> > num_ctl = num_intf.
> >
>
> num_ctl will be = num_intf, but what I was trying to understand here is
> that , previously this condition was making sure that we have a ctl
> assigned for each physical encoder which is actually a requirement for
> the display pipeline. If we assigned a hw_ctl for one phys encoder and
> not the other, its an error.
>
> But on closer look, I think even your check will catch that.
>
>
> >>
> >> Will it be just easier to bring back the NULL assignment at the top?
> >>
> >> struct dpu_hw_blk *hw_ctl[MAX_CHANNELS_PER_ENC] = { NULL };
> >>
> >> I also see the same issue for other blocks such as hw_dsc, hw_lm
> >
> > Other blocks loop properly up to the num_resource. I'd prefer to drop
> > the NULL init from the DSPP init and use num_dspp instead.
> >
>
> Overall, I think the purpose of NULL init was to make sure that before
> we call to_dpu_hw_***() macros, we have a valid hw_*.
>
> We could use either num_* or the hw_* as both are returned by RM.
>
> One side-note here is with a proper NULL hw_ctl is that the consumers of
> hw_ctl should also be able to check for NULL correctly.
The problem of the NULL checks is that it's too tempting to perform a
NULL check after to_dpu_hw_ctl conversion. However it's not safe to
pass NULL pointer to such functions: there is no guarantee that
conversion will return NULL if it gets passed the NULL pointer.
> So for example dpu_encoder_phys layers use if (!phys->hw_ctl) checks but
> today we do not set phys->hw_ctl to NULL correctly.
>
> Do you think that instead of the return statements, we should do
> something like
>
> dpu_enc->hw_ctl = i < num_ctl ?
> to_dpu_hw_ctl(hw_ctl[i]) : NULL;
Yeah, why not.
Generally, I think we should stop storing the state-related data in
the non-state structures. Hopefully I'll have time for that at some
point later on.
>
>
> But this will need the NULL init back.
It doesn't, as you have the comparison.
>
> >>
> >>>DPU_ERROR_ENC(dpu_enc,
> >>>"no ctl block assigned at idx: %d\n", i);
> >>>return;
> >>>
> >
> >
> >
--
With best wishes
Dmitry
Re: [PATCH v5 02/16] drm/msm/dpu: fix error condition in dpu_encoder_virt_atomic_mode_set
On Fri, 12 Jul 2024 at 22:41, Abhinav Kumar wrote:
> On 6/24/2024 2:13 PM, Dmitry Baryshkov wrote:
> > The commit b954fa6baaca ("drm/msm/dpu: Refactor rm iterator") removed
> > zero-init of the hw_ctl array, but didn't change the error condition,
> > that checked for hw_ctl[i] being NULL. Use indices check instead.
> >
> > Fixes: b954fa6baaca ("drm/msm/dpu: Refactor rm iterator")
> > Signed-off-by: Dmitry Baryshkov
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > index 5d205e09cf45..7613005fbfea 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > @@ -1186,7 +1186,7 @@ static void :tag(struct drm_encoder *drm_enc,
> > return;
> > }
> >
> > - if (!hw_ctl[i]) {
> > + if (i >= num_ctl) {
>
> This is not very clear to me.
>
> How will we hit this condition? I dont see i going beyond 1 in this loop
> and neither should num_ctl
Why? the driver doesn't support flushing through a single CTL, so
num_ctl = num_intf.
>
> Will it be just easier to bring back the NULL assignment at the top?
>
> struct dpu_hw_blk *hw_ctl[MAX_CHANNELS_PER_ENC] = { NULL };
>
> I also see the same issue for other blocks such as hw_dsc, hw_lm
Other blocks loop properly up to the num_resource. I'd prefer to drop
the NULL init from the DSPP init and use num_dspp instead.
>
> > DPU_ERROR_ENC(dpu_enc,
> > "no ctl block assigned at idx: %d\n", i);
> > return;
> >
--
With best wishes
Dmitry
Re: [PATCH 1/2] clk: qocm: add qcom_cc_map_norequest
On Thu, 11 Jul 2024 at 03:04, Stephen Boyd wrote: > > Quoting Dmitry Baryshkov (2024-07-10 16:32:18) > > On Tue, 9 Jul 2024 at 01:30, Stephen Boyd wrote: > > > > > > Quoting Dmitry Baryshkov (2024-06-27 22:20:22) > > > > The GPU clock controllers use memory region that is a part of the GMU's > > > > memory region. Add qcom_cc_map_norequest() to be used by GPUCC, so that > > > > GPU driver can use devm_ioremap_resource for GMU resources. > > > > > > Why does GMU map the gpu clk controller? Does it use those registers? We > > > don't want to allow two different drivers to map the same region because > > > then they don't coordinate and write over things. > > > > It's not that GMU maps gpu CC separately. It looks more like gpucc is > > a part of the GMU address space. I think GMU manages some of the > > clocks or GDSCs directly. > > > > I imagine GMU is a collection of stuff, so the register range is large > because it's basically a subsystem unto itself. Can the range in DT be > split up, or changed so that different devices within GMU are split out? No, we have to remain compatible with existing DT. It's not a problem of a single new platform, the issue has always been present there. > Or maybe the gpu clk controller can be made into a child of some GMU > node, where the GMU node has a driver that populates devices that match > drivers in different subsystems. Well... Technically yes, but this brings another pack of issues. There is no separate GMU driver, so we will likely have a chicken-and-egg problem, as probing of the GPU driver will also create the gpucc device which is further used by the GPU. -- With best wishes Dmitry
Re: [PATCH 1/2] clk: qocm: add qcom_cc_map_norequest
On Tue, 9 Jul 2024 at 01:30, Stephen Boyd wrote: > > Quoting Dmitry Baryshkov (2024-06-27 22:20:22) > > The GPU clock controllers use memory region that is a part of the GMU's > > memory region. Add qcom_cc_map_norequest() to be used by GPUCC, so that > > GPU driver can use devm_ioremap_resource for GMU resources. > > Why does GMU map the gpu clk controller? Does it use those registers? We > don't want to allow two different drivers to map the same region because > then they don't coordinate and write over things. It's not that GMU maps gpu CC separately. It looks more like gpucc is a part of the GMU address space. I think GMU manages some of the clocks or GDSCs directly. -- With best wishes Dmitry
Re: [PATCH 2/2] drm/msm/dpu: don't play tricks with debug macros
On Tue, 9 Jul 2024 at 22:39, Abhinav Kumar wrote:
>
>
>
> On 7/9/2024 6:48 AM, Dmitry Baryshkov wrote:
> > DPU debugging macros need to be converted to a proper drm_debug_*
> > macros, however this is a going an intrusive patch, not suitable for a
> > fix. Wire DPU_DEBUG and DPU_DEBUG_DRIVER to always use DRM_DEBUG_DRIVER
> > to make sure that DPU debugging messages always end up in the drm debug
> > messages and are controlled via the usual drm.debug mask.
> >
>
> These macros have been deprecated, is this waht you meant by the
> conversion to proper drm_debug_*?
Yes. Drop the driver-specific wrappers where they don't make sense.
Use sensible format strings in the cases where it actually does (like
VIDENC or _PLANE)
>
> /* NOTE: this is deprecated in favor of drm_dbg(NULL, ...). */
> #define DRM_DEBUG_DRIVER(fmt, ...) \
> __drm_dbg(DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
>
> I think all that this macro was doing was to have appropriate DRM_UT_*
> macros enabled before calling the corresponding DRM_DEBUG_* macros. But
> I think what was incorrect here is for DPU_DEBUG, we could have used
> DRM_UT_CORE instead of DRM_UT_KMS.
It pretty much tries to overplay the existing drm debugging mechanism
by either sending the messages to the DRM channel or just using
pr_debug. With DYNAMIC_DEBUG being disabled pr_debug is just an empty
macro, so all the messages can end up in /dev/null. We should not be
trying to be too smart, using standard DRM_DEBUG_DRIVER should be
enough. This way all driver-related messages are controlled by
drm.debug including or excluding the 0x02 bit.
>
> And DRM_DEBUG_DRIVER should have been used instead of DRM_ERROR.
>
> Was this causing the issue of the prints not getting enabled?
I pretty much think so.
>
> > Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support")
> > Signed-off-by: Dmitry Baryshkov
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 14 ++
> > 1 file changed, 2 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> > index e2adc937ea63..935ff6fd172c 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
> > @@ -31,24 +31,14 @@
> >* @fmt: Pointer to format string
> >*/
> > #define DPU_DEBUG(fmt, ...)
> > \
> > - do { \
> > - if (drm_debug_enabled(DRM_UT_KMS)) \
> > - DRM_DEBUG(fmt, ##__VA_ARGS__); \
> > - else \
> > - pr_debug(fmt, ##__VA_ARGS__); \
> > - } while (0)
> > + DRM_DEBUG_DRIVER(fmt, ##__VA_ARGS__)
> >
> > /**
> >* DPU_DEBUG_DRIVER - macro for hardware driver logging
> >* @fmt: Pointer to format string
> >*/
> > #define DPU_DEBUG_DRIVER(fmt, ...)
> > \
> > - do { \
> > - if (drm_debug_enabled(DRM_UT_DRIVER)) \
> > - DRM_ERROR(fmt, ##__VA_ARGS__); \
> > - else \
> > - pr_debug(fmt, ##__VA_ARGS__); \
> > - } while (0)
> > + DRM_DEBUG_DRIVER(fmt, ##__VA_ARGS__)
> >
> > #define DPU_ERROR(fmt, ...) pr_err("[dpu error]" fmt, ##__VA_ARGS__)
> > #define DPU_ERROR_RATELIMITED(fmt, ...) pr_err_ratelimited("[dpu error]"
> > fmt, ##__VA_ARGS__)
> >
--
With best wishes
Dmitry
[PATCH 2/2] drm/msm/dpu: don't play tricks with debug macros
DPU debugging macros need to be converted to a proper drm_debug_*
macros, however this is a going an intrusive patch, not suitable for a
fix. Wire DPU_DEBUG and DPU_DEBUG_DRIVER to always use DRM_DEBUG_DRIVER
to make sure that DPU debugging messages always end up in the drm debug
messages and are controlled via the usual drm.debug mask.
Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support")
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 14 ++
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index e2adc937ea63..935ff6fd172c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -31,24 +31,14 @@
* @fmt: Pointer to format string
*/
#define DPU_DEBUG(fmt, ...)\
- do { \
- if (drm_debug_enabled(DRM_UT_KMS)) \
- DRM_DEBUG(fmt, ##__VA_ARGS__); \
- else \
- pr_debug(fmt, ##__VA_ARGS__); \
- } while (0)
+ DRM_DEBUG_DRIVER(fmt, ##__VA_ARGS__)
/**
* DPU_DEBUG_DRIVER - macro for hardware driver logging
* @fmt: Pointer to format string
*/
#define DPU_DEBUG_DRIVER(fmt, ...) \
- do { \
- if (drm_debug_enabled(DRM_UT_DRIVER)) \
- DRM_ERROR(fmt, ##__VA_ARGS__); \
- else \
- pr_debug(fmt, ##__VA_ARGS__); \
- } while (0)
+ DRM_DEBUG_DRIVER(fmt, ##__VA_ARGS__)
#define DPU_ERROR(fmt, ...) pr_err("[dpu error]" fmt, ##__VA_ARGS__)
#define DPU_ERROR_RATELIMITED(fmt, ...) pr_err_ratelimited("[dpu error]" fmt,
##__VA_ARGS__)
--
2.39.2
[PATCH 1/2] drm/msm/dpu1: don't choke on disabling the writeback connector
In order to prevent any errors on connector being disabled, move the
state->crtc check upfront. This should fix the issues during suspend
when the writeback connector gets forcebly disabled.
Fixes: 71174f362d67 ("drm/msm/dpu: move writeback's atomic_check to
dpu_writeback.c")
Cc: sta...@vger.kernel.org
Reported-by: Leonard Lausen
Closes: https://gitlab.freedesktop.org/drm/msm/-/issues/57
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c
index 16f144cbc0c9..5c172bcf3419 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c
@@ -39,6 +39,13 @@ static int dpu_wb_conn_atomic_check(struct drm_connector
*connector,
DPU_DEBUG("[atomic_check:%d]\n", connector->base.id);
+ crtc = conn_state->crtc;
+ if (!crtc)
+ return 0;
+
+ if (!conn_state->writeback_job || !conn_state->writeback_job->fb)
+ return 0;
+
if (!conn_state || !conn_state->connector) {
DPU_ERROR("invalid connector state\n");
return -EINVAL;
@@ -47,13 +54,6 @@ static int dpu_wb_conn_atomic_check(struct drm_connector
*connector,
return -EINVAL;
}
- crtc = conn_state->crtc;
- if (!crtc)
- return 0;
-
- if (!conn_state->writeback_job || !conn_state->writeback_job->fb)
- return 0;
-
crtc_state = drm_atomic_get_crtc_state(state, crtc);
if (IS_ERR(crtc_state))
return PTR_ERR(crtc_state);
--
2.39.2
[PATCH 0/2] drm/msm/dpu: two fixes targeting 6.11
Leonard Lausen reported an issue with suspend/resume of the sc7180 devices. Fix the WB atomic check, which caused the issue. Also make sure that DPU debugging logs are always directed to the drm_debug / DRIVER so that usual drm.debug masks work in an expected way. Signed-off-by: Dmitry Baryshkov --- Dmitry Baryshkov (2): drm/msm/dpu1: don't choke on disabling the writeback connector drm/msm/dpu: don't play tricks with debug macros drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 14 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_writeback.c | 14 +++--- 2 files changed, 9 insertions(+), 19 deletions(-) --- base-commit: 0b58e108042b0ed28a71cd7edf517555b233 change-id: 20240709-dpu-fix-wb-6cd57e3eb182 Best regards, -- Dmitry Baryshkov
Re: [PATCH v2] drm/msm: add msm8998 hdmi phy/pll support
On Tue, 9 Jul 2024 at 00:27, Konrad Dybcio wrote: > > On 8.07.2024 2:49 PM, Dmitry Baryshkov wrote: > > On Mon, 8 Jul 2024 at 14:07, Marc Gonzalez wrote: > >> > >> On 05/07/2024 16:34, Dmitry Baryshkov wrote: > > [...] > > >>> I'm not going to check the math, but it looks pretty close to what we > >>> have for msm8996. > >> > >> What is the consequence of this? > > > > That I won't check the math :-D > > Dmitry is trying to say that you should check whether the calculations > are the same or almost the same as in the 8996 driver, and if so, try > to commonize the code between the two Not quite :-D They are slightly different. More importantly, this is a different version of QMP PHY. So, it's not really worth merging the code. Earlier on I pasted the patchset to move all HDMI PHY drivers to drivers/phy/qualcomm. I plan to integrate msm8998 support into that patchset (this should not be delaying this patch though). But I don't want to commonize the HDMI QMP PHY code before somebody implements support for the third version of QMP HDMI PHYs, the one that is found on msm8992/94. -- With best wishes Dmitry
Re: [PATCH v2] drm/msm: add msm8998 hdmi phy/pll support
On Mon, 8 Jul 2024 at 14:07, Marc Gonzalez wrote:
>
> On 05/07/2024 16:34, Dmitry Baryshkov wrote:
>
> > On Thu, Jul 04, 2024 at 06:45:36PM GMT, Marc Gonzalez wrote:
> >
> >> From: Arnaud Vrac
> >>
> >> Ported from the downstream driver.
> >
> > Please write some sensible commit message.
>
> Here's an attempt at expanding the commit message:
>
> """
> This code adds support for the HDMI PHY block in the MSM8998.
> It is a copy & paste of the vendor driver downstream:
> https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/drivers/clk/msm/mdss/mdss-hdmi-pll-8998.c
> """
Add support for the HDMI PHY as present on the Qualcomm MSM8998
platform. The code is mostly c of the vendor code from msm-4.4,
kernel.lnx.4.4.r38-rel.
>
>
> >> drivers/gpu/drm/msm/Makefile | 1 +
> >> drivers/gpu/drm/msm/hdmi/hdmi.c| 1 +
> >> drivers/gpu/drm/msm/hdmi/hdmi.h| 8 +
> >> drivers/gpu/drm/msm/hdmi/hdmi_phy.c| 5 +
> >> drivers/gpu/drm/msm/hdmi/hdmi_phy_8998.c | 789
> >> +
> >> drivers/gpu/drm/msm/registers/display/hdmi.xml | 89 +++
> >> 6 files changed, 893 insertions(+)
> >
> > - Missing changelog
>
> - Rebase onto v6.10
> - Move drivers/gpu/drm/msm/hdmi/hdmi.xml.h to
> drivers/gpu/drm/msm/registers/display/hdmi.xml
> - Add copyright attribution
> - Remove all dead/debug/temporary code
>
> > - Missing a pointer to bindings. Ideally bindings should come together with
> > the driver.
>
> "qcom,hdmi-phy-8998" is defined in "HDMI TX support in msm8998" series (Acked
> by Rob Herring & Vinod Koul)
This (and the link to lore) ideally should be a part of the cover
letter or the comment below '---' in the patch.
>
> > I'm not going to check the math, but it looks pretty close to what we
> > have for msm8996.
>
> What is the consequence of this?
That I won't check the math :-D
>
>
> >> +static const char * const hdmi_phy_8998_reg_names[] = {
> >> +"vdda-pll",
> >> +"vdda-phy",
> >
> > Unless you have a strong reason to, please use vcca and vddio here, so
> > that we don't have unnecessary conditionals in schema.
>
> The vendor code uses vddio & vcca for msm8996;
> vdda-pll & vdda-phy for msm8998.
>
> Which is vcca? Which is vddio?
vddio = vdda-phy (1.8V)
vcca = vdda-pll (lower voltage)
> https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/arch/arm/boot/dts/qcom/msm8996-mdss-pll.dtsi
> https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/caf_migration/kernel.lnx.4.4.r38-rel/arch/arm/boot/dts/qcom/msm8998-mdss-pll.dtsi#L121-172
--
With best wishes
Dmitry
Re: [PATCH v2] drm/msm: add msm8998 hdmi phy/pll support
On Thu, Jul 04, 2024 at 06:45:36PM GMT, Marc Gonzalez wrote:
> From: Arnaud Vrac
>
> Ported from the downstream driver.
Please write some sensible commit message.
>
> Signed-off-by: Arnaud Vrac
> Signed-off-by: Marc Gonzalez
> ---
> drivers/gpu/drm/msm/Makefile | 1 +
> drivers/gpu/drm/msm/hdmi/hdmi.c| 1 +
> drivers/gpu/drm/msm/hdmi/hdmi.h| 8 +
> drivers/gpu/drm/msm/hdmi/hdmi_phy.c| 5 +
> drivers/gpu/drm/msm/hdmi/hdmi_phy_8998.c | 789
> +
> drivers/gpu/drm/msm/registers/display/hdmi.xml | 89 +++
> 6 files changed, 893 insertions(+)
- Missing changelog
- Missing a pointer to bindings. Ideally bindings should come together
with the driver.
I'm not going to check the math, but it looks pretty close to what we
have for msm8996.
> +
> +static const char * const hdmi_phy_8998_reg_names[] = {
> + "vdda-pll",
> + "vdda-phy",
Unless you have a strong reason to, please use vcca and vddio here, so
that we don't have unnecessary conditionals in schema.
--
With best wishes
Dmitry
Re: [PATCH v2] drm/msm: Fix incorrect file name output in adreno_request_fw()
On Fri, 5 Jul 2024 at 12:15, Aleksandr Mishin wrote:
>
> In adreno_request_fw() when debugging information is printed to the log
> after firmware load, an incorrect filename is printed. 'newname' is used
> instead of 'fwname', so prefix "qcom/" is being added to filename.
> Looks like "copy-paste" mistake.
>
> Fix this mistake by replacing 'newname' with 'fwname'.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Fixes: 2c41ef1b6f7d ("drm/msm/adreno: deal with linux-firmware fw paths")
> Signed-off-by: Aleksandr Mishin
> ---
> v1->v2: Fix incorrect 'Fixes' tag
>
> drivers/gpu/drm/msm/adreno/adreno_gpu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Reviewed-by: Dmitry Baryshkov
--
With best wishes
Dmitry
[PATCH v6] drm/display: split DSC helpers from DP helpers
Currently the DRM DSC functions are selected by the DRM_DISPLAY_DP_HELPER Kconfig symbol. This is not optimal, since the DSI code (both panel and host drivers) end up selecting the seemingly irrelevant DP helpers. Split the DSC code to be guarded by the separate DRM_DISPLAY_DSC_HELPER Kconfig symbol. Reviewed-by: Jessica Zhang Reviewed-by: Marijn Suijten Acked-by: Rodrigo Vivi #i915 Signed-off-by: Dmitry Baryshkov --- Changes in v6: - Moved the Makefile entry to follow the sorting order (Thomas Zimmermann) - Link to v5: https://lore.kernel.org/r/20240623-panel-sw43408-fix-v5-1-5401ab61e...@linaro.org Changes in v5: - Drop applied patches - Link to v4: https://lore.kernel.org/r/20240528-panel-sw43408-fix-v4-0-330b42445...@linaro.org Changes in v4: - Reoder patches so that fixes come first, to be able to land them to drm-misc-fixes - Link to v3: https://lore.kernel.org/r/20240522-panel-sw43408-fix-v3-0-6902285ad...@linaro.org Changes in v3: - Split DRM_DISPLAY_DSC_HELPER from DRM_DISPLAY_DP_HELPER - Added missing Fixes tags - Link to v2: https://lore.kernel.org/r/20240510-panel-sw43408-fix-v2-0-d1ef91ee1...@linaro.org Changes in v2: - use SELECT instead of DEPEND to follow the reverted Kconfig changes - Link to v1: https://lore.kernel.org/r/20240420-panel-sw43408-fix-v1-0-b282ff725...@linaro.org --- drivers/gpu/drm/amd/amdgpu/Kconfig | 1 + drivers/gpu/drm/display/Kconfig| 6 ++ drivers/gpu/drm/display/Makefile | 5 +++-- drivers/gpu/drm/i915/Kconfig | 1 + drivers/gpu/drm/msm/Kconfig| 1 + drivers/gpu/drm/panel/Kconfig | 6 +++--- 6 files changed, 15 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/Kconfig b/drivers/gpu/drm/amd/amdgpu/Kconfig index 0051fb1b437f..fc3237da8090 100644 --- a/drivers/gpu/drm/amd/amdgpu/Kconfig +++ b/drivers/gpu/drm/amd/amdgpu/Kconfig @@ -6,6 +6,7 @@ config DRM_AMDGPU depends on !UML select FW_LOADER select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_DSC_HELPER select DRM_DISPLAY_HDMI_HELPER select DRM_DISPLAY_HDCP_HELPER select DRM_DISPLAY_HELPER diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig index 479e62690d75..a2e42014ffe0 100644 --- a/drivers/gpu/drm/display/Kconfig +++ b/drivers/gpu/drm/display/Kconfig @@ -59,6 +59,12 @@ config DRM_DISPLAY_DP_TUNNEL_STATE_DEBUG If in doubt, say "N". +config DRM_DISPLAY_DSC_HELPER + bool + depends on DRM_DISPLAY_HELPER + help + DRM display helpers for VESA DSC (used by DSI and DisplayPort). + config DRM_DISPLAY_HDCP_HELPER bool depends on DRM_DISPLAY_HELPER diff --git a/drivers/gpu/drm/display/Makefile b/drivers/gpu/drm/display/Makefile index 629df2f4d322..a023f72fa139 100644 --- a/drivers/gpu/drm/display/Makefile +++ b/drivers/gpu/drm/display/Makefile @@ -6,10 +6,11 @@ drm_display_helper-y := drm_display_helper_mod.o drm_display_helper-$(CONFIG_DRM_DISPLAY_DP_HELPER) += \ drm_dp_dual_mode_helper.o \ drm_dp_helper.o \ - drm_dp_mst_topology.o \ - drm_dsc_helper.o + drm_dp_mst_topology.o drm_display_helper-$(CONFIG_DRM_DISPLAY_DP_TUNNEL) += \ drm_dp_tunnel.o +drm_display_helper-$(CONFIG_DRM_DISPLAY_DSC_HELPER) += \ + drm_dsc_helper.o drm_display_helper-$(CONFIG_DRM_DISPLAY_HDCP_HELPER) += drm_hdcp_helper.o drm_display_helper-$(CONFIG_DRM_DISPLAY_HDMI_HELPER) += \ drm_hdmi_helper.o \ diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index faa253b27664..db400aad88fa 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -11,6 +11,7 @@ config DRM_I915 select SHMEM select TMPFS select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_DSC_HELPER select DRM_DISPLAY_HDCP_HELPER select DRM_DISPLAY_HDMI_HELPER select DRM_DISPLAY_HELPER diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig index 26a4c71da63a..420385c47193 100644 --- a/drivers/gpu/drm/msm/Kconfig +++ b/drivers/gpu/drm/msm/Kconfig @@ -112,6 +112,7 @@ config DRM_MSM_DSI depends on DRM_MSM select DRM_PANEL select DRM_MIPI_DSI + select DRM_DISPLAY_DSC_HELPER default y help Choose this option if you have a need for MIPI DSI connector diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig index 9f49b0189d3b..dac01ade7e2e 100644 --- a/drivers/gpu/drm/panel/Kconfig +++ b/drivers/gpu/drm/panel/Kconfig @@ -369,7 +369,7 @@ config DRM_PANEL_LG_SW43408 depends on OF depends on DRM_MIPI_DSI depends on BACKLIGHT_CLASS_DEVICE - select DRM_DISPLAY_DP_HELPER + select DRM_DISPLAY_DSC_HELPER select DRM_DISPLAY_HELPER help Say Y here if you want to enable support for LG sw43408 panel. @@ -578,7 +578,7 @@ config DRM_PANEL_RAYDIUM_RM692E5 depends on OF depends on DR
Re: [PATCH v3 3/4] drm/bridge-connector: move to DRM_DISPLAY_HELPER module
On Thu, 4 Jul 2024 at 15:54, Maxime Ripard wrote:
>
> On Tue, Jul 02, 2024 at 12:48:54PM GMT, Dmitry Baryshkov wrote:
> > drm_bridge_connector is a "leaf" driver, belonging to the display
> > helper, rather than the "CRTC" drm_kms_helper module. Move the driver
> > to the drm/display and add necessary Kconfig selection clauses.
> >
> > Suggested-by: Maxime Ripard
> > Signed-off-by: Dmitry Baryshkov
> > ---
> > drivers/gpu/drm/Makefile | 1 -
> > drivers/gpu/drm/bridge/Kconfig | 1 +
> > drivers/gpu/drm/display/Kconfig | 6 ++
> > drivers/gpu/drm/display/Makefile | 2 ++
> > drivers/gpu/drm/{ => display}/drm_bridge_connector.c | 0
> > drivers/gpu/drm/imx/dcss/Kconfig | 2 ++
> > drivers/gpu/drm/imx/lcdc/Kconfig | 2 ++
> > drivers/gpu/drm/ingenic/Kconfig | 2 ++
> > drivers/gpu/drm/kmb/Kconfig | 2 ++
> > drivers/gpu/drm/mediatek/Kconfig | 2 ++
> > drivers/gpu/drm/meson/Kconfig| 2 ++
> > drivers/gpu/drm/msm/Kconfig | 1 +
> > drivers/gpu/drm/omapdrm/Kconfig | 2 ++
> > drivers/gpu/drm/renesas/rcar-du/Kconfig | 2 ++
> > drivers/gpu/drm/renesas/rz-du/Kconfig| 2 ++
> > drivers/gpu/drm/renesas/shmobile/Kconfig | 2 ++
> > drivers/gpu/drm/rockchip/Kconfig | 4
> > drivers/gpu/drm/tegra/Kconfig| 1 +
> > drivers/gpu/drm/tidss/Kconfig| 2 ++
> > drivers/gpu/drm/xlnx/Kconfig | 1 +
> > 20 files changed, 38 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> > index 68cc9258ffc4..fa432a1ac9e2 100644
> > --- a/drivers/gpu/drm/Makefile
> > +++ b/drivers/gpu/drm/Makefile
> > @@ -128,7 +128,6 @@ obj-$(CONFIG_DRM_TTM_HELPER) += drm_ttm_helper.o
> > drm_kms_helper-y := \
> > drm_atomic_helper.o \
> > drm_atomic_state_helper.o \
> > - drm_bridge_connector.o \
> > drm_crtc_helper.o \
> > drm_damage_helper.o \
> > drm_encoder_slave.o \
> > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> > index c621be1a99a8..3eb955333c80 100644
> > --- a/drivers/gpu/drm/bridge/Kconfig
> > +++ b/drivers/gpu/drm/bridge/Kconfig
> > @@ -390,6 +390,7 @@ config DRM_TI_SN65DSI86
> > depends on OF
> > select DRM_DISPLAY_DP_HELPER
> > select DRM_DISPLAY_HELPER
> > + select DRM_BRIDGE_CONNECTOR
> > select DRM_KMS_HELPER
> > select REGMAP_I2C
> > select DRM_PANEL
> > diff --git a/drivers/gpu/drm/display/Kconfig
> > b/drivers/gpu/drm/display/Kconfig
> > index 479e62690d75..1a192a45961b 100644
> > --- a/drivers/gpu/drm/display/Kconfig
> > +++ b/drivers/gpu/drm/display/Kconfig
> > @@ -6,6 +6,12 @@ config DRM_DISPLAY_HELPER
> > help
> > DRM helpers for display adapters.
> >
> > +config DRM_BRIDGE_CONNECTOR
> > + bool
> > + depends on DRM && DRM_BRIDGE && DRM_DISPLAY_HELPER
> > + help
> > + DRM connector implementation terminating DRM bridge chains.
> > +
>
> Is there any reason to put it in there instead of under DRM_BRIDGE like
> DRM_PANEL_BRIDGE?
DRM_PANEL_ BRIDGE is a bridge in the end. DRM_BRIDGE_CONNECTOR is not.
That's why I thought that drm/display is a better way to go.
--
With best wishes
Dmitry
Re: [PATCH v3 4/4] drm/bridge-connector: reset the HDMI connector state
On Thu, 4 Jul 2024 at 15:56, Maxime Ripard wrote:
>
> hi,
>
> On Tue, Jul 02, 2024 at 12:48:55PM GMT, Dmitry Baryshkov wrote:
> > On HDMI connectors which use drm_bridge_connector and DRM_BRIDGE_OP_HDMI
> > IGT chokes on the max_bpc property in several kms_properties tests due
> > to the the drm_bridge_connector failing to reset HDMI-related
> > properties.
> >
> > Call __drm_atomic_helper_connector_hdmi_reset() if the
> > drm_bridge_connector has bridge_hdmi.
> >
> > It is impossible to call this function from HDMI bridges, there is is no
> > function that corresponds to the drm_connector_funcs::reset().
> >
> > Fixes: 6b4468b0c6ba ("drm/bridge-connector: implement glue code for HDMI
> > connector")
> > Signed-off-by: Dmitry Baryshkov
> > ---
> > drivers/gpu/drm/display/Kconfig| 1 +
> > drivers/gpu/drm/display/drm_bridge_connector.c | 13 -
> > 2 files changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/display/Kconfig
> > b/drivers/gpu/drm/display/Kconfig
> > index 1a192a45961b..bfd025f8c7b5 100644
> > --- a/drivers/gpu/drm/display/Kconfig
> > +++ b/drivers/gpu/drm/display/Kconfig
> > @@ -9,6 +9,7 @@ config DRM_DISPLAY_HELPER
> > config DRM_BRIDGE_CONNECTOR
> > bool
> > depends on DRM && DRM_BRIDGE && DRM_DISPLAY_HELPER
> > + select DRM_DISPLAY_HDMI_STATE_HELPER
>
> Sorry, I notice it on that patch, but it's really on the previous one:
> both DRM_BRIDGE and DRM_DISPLAY_HELPER are meant to be selected, not
> depended on.
I think they are selected by the 'driver' drivers, but they are
depended upon by the sub-devices. So all bridges depend on DRM_BRIDGE
(either directly or via the menu entry).
All drm/display modules depend on DRM_DISPLAY_HELPER. I agree that it
might be worth changing the second part (most likely in a separate
series, ok), but I'm sure that the DRM_BRIDGE part is correct.
>
> Otherwise,
>
> Reviewed-by: Maxime Ripard
>
> Maxime
--
With best wishes
Dmitry
Re: [PATCH] drm/msm: Fix incorrect file name output in adreno_request_fw()
On 04/07/2024 12:30, Aleksandr Mishin wrote:
In adreno_request_fw() when debugging information is printed to the log
after firmware load, an incorrect filename is printed. 'newname' is used
instead of 'fwname', so prefix "qcom/" is being added to filename.
Looks like "copy-paste" mistake.
Fix this mistake by replacing 'newname' with 'fwname'.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Fixes: 9fe041f6fdfe ("drm/msm: Add msm_gem_get_and_pin_iova()")
Fixes tag is incorrect, LGTM otherwise.
Signed-off-by: Aleksandr Mishin
---
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 074fb498706f..0bb7d66047f8 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -475,7 +475,7 @@ adreno_request_fw(struct adreno_gpu *adreno_gpu, const char
*fwname)
ret = request_firmware_direct(, fwname, drm->dev);
if (!ret) {
DRM_DEV_INFO(drm->dev, "loaded %s from legacy
location\n",
- newname);
+ fwname);
adreno_gpu->fwloc = FW_LOCATION_LEGACY;
goto out;
} else if (adreno_gpu->fwloc != FW_LOCATION_UNKNOWN) {
--
With best wishes
Dmitry
[PATCH v3 4/4] drm/bridge-connector: reset the HDMI connector state
On HDMI connectors which use drm_bridge_connector and DRM_BRIDGE_OP_HDMI
IGT chokes on the max_bpc property in several kms_properties tests due
to the the drm_bridge_connector failing to reset HDMI-related
properties.
Call __drm_atomic_helper_connector_hdmi_reset() if the
drm_bridge_connector has bridge_hdmi.
It is impossible to call this function from HDMI bridges, there is is no
function that corresponds to the drm_connector_funcs::reset().
Fixes: 6b4468b0c6ba ("drm/bridge-connector: implement glue code for HDMI
connector")
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/display/Kconfig| 1 +
drivers/gpu/drm/display/drm_bridge_connector.c | 13 -
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig
index 1a192a45961b..bfd025f8c7b5 100644
--- a/drivers/gpu/drm/display/Kconfig
+++ b/drivers/gpu/drm/display/Kconfig
@@ -9,6 +9,7 @@ config DRM_DISPLAY_HELPER
config DRM_BRIDGE_CONNECTOR
bool
depends on DRM && DRM_BRIDGE && DRM_DISPLAY_HELPER
+ select DRM_DISPLAY_HDMI_STATE_HELPER
help
DRM connector implementation terminating DRM bridge chains.
diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c
b/drivers/gpu/drm/display/drm_bridge_connector.c
index 0869b663f17e..7ebb35438459 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -216,8 +216,19 @@ static void drm_bridge_connector_debugfs_init(struct
drm_connector *connector,
}
}
+static void drm_bridge_connector_reset(struct drm_connector *connector)
+{
+ struct drm_bridge_connector *bridge_connector =
+ to_drm_bridge_connector(connector);
+
+ drm_atomic_helper_connector_reset(connector);
+ if (bridge_connector->bridge_hdmi)
+ __drm_atomic_helper_connector_hdmi_reset(connector,
+connector->state);
+}
+
static const struct drm_connector_funcs drm_bridge_connector_funcs = {
- .reset = drm_atomic_helper_connector_reset,
+ .reset = drm_bridge_connector_reset,
.detect = drm_bridge_connector_detect,
.fill_modes = drm_helper_probe_single_connector_modes,
.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
--
2.39.2
[PATCH v3 3/4] drm/bridge-connector: move to DRM_DISPLAY_HELPER module
drm_bridge_connector is a "leaf" driver, belonging to the display
helper, rather than the "CRTC" drm_kms_helper module. Move the driver
to the drm/display and add necessary Kconfig selection clauses.
Suggested-by: Maxime Ripard
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/Makefile | 1 -
drivers/gpu/drm/bridge/Kconfig | 1 +
drivers/gpu/drm/display/Kconfig | 6 ++
drivers/gpu/drm/display/Makefile | 2 ++
drivers/gpu/drm/{ => display}/drm_bridge_connector.c | 0
drivers/gpu/drm/imx/dcss/Kconfig | 2 ++
drivers/gpu/drm/imx/lcdc/Kconfig | 2 ++
drivers/gpu/drm/ingenic/Kconfig | 2 ++
drivers/gpu/drm/kmb/Kconfig | 2 ++
drivers/gpu/drm/mediatek/Kconfig | 2 ++
drivers/gpu/drm/meson/Kconfig| 2 ++
drivers/gpu/drm/msm/Kconfig | 1 +
drivers/gpu/drm/omapdrm/Kconfig | 2 ++
drivers/gpu/drm/renesas/rcar-du/Kconfig | 2 ++
drivers/gpu/drm/renesas/rz-du/Kconfig| 2 ++
drivers/gpu/drm/renesas/shmobile/Kconfig | 2 ++
drivers/gpu/drm/rockchip/Kconfig | 4
drivers/gpu/drm/tegra/Kconfig| 1 +
drivers/gpu/drm/tidss/Kconfig| 2 ++
drivers/gpu/drm/xlnx/Kconfig | 1 +
20 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 68cc9258ffc4..fa432a1ac9e2 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -128,7 +128,6 @@ obj-$(CONFIG_DRM_TTM_HELPER) += drm_ttm_helper.o
drm_kms_helper-y := \
drm_atomic_helper.o \
drm_atomic_state_helper.o \
- drm_bridge_connector.o \
drm_crtc_helper.o \
drm_damage_helper.o \
drm_encoder_slave.o \
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index c621be1a99a8..3eb955333c80 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -390,6 +390,7 @@ config DRM_TI_SN65DSI86
depends on OF
select DRM_DISPLAY_DP_HELPER
select DRM_DISPLAY_HELPER
+ select DRM_BRIDGE_CONNECTOR
select DRM_KMS_HELPER
select REGMAP_I2C
select DRM_PANEL
diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig
index 479e62690d75..1a192a45961b 100644
--- a/drivers/gpu/drm/display/Kconfig
+++ b/drivers/gpu/drm/display/Kconfig
@@ -6,6 +6,12 @@ config DRM_DISPLAY_HELPER
help
DRM helpers for display adapters.
+config DRM_BRIDGE_CONNECTOR
+ bool
+ depends on DRM && DRM_BRIDGE && DRM_DISPLAY_HELPER
+ help
+ DRM connector implementation terminating DRM bridge chains.
+
config DRM_DISPLAY_DP_AUX_BUS
tristate
depends on DRM
diff --git a/drivers/gpu/drm/display/Makefile b/drivers/gpu/drm/display/Makefile
index 629df2f4d322..fbb9d2b8acd4 100644
--- a/drivers/gpu/drm/display/Makefile
+++ b/drivers/gpu/drm/display/Makefile
@@ -3,6 +3,8 @@
obj-$(CONFIG_DRM_DISPLAY_DP_AUX_BUS) += drm_dp_aux_bus.o
drm_display_helper-y := drm_display_helper_mod.o
+drm_display_helper-$(CONFIG_DRM_BRIDGE_CONNECTOR) += \
+ drm_bridge_connector.o
drm_display_helper-$(CONFIG_DRM_DISPLAY_DP_HELPER) += \
drm_dp_dual_mode_helper.o \
drm_dp_helper.o \
diff --git a/drivers/gpu/drm/drm_bridge_connector.c
b/drivers/gpu/drm/display/drm_bridge_connector.c
similarity index 100%
rename from drivers/gpu/drm/drm_bridge_connector.c
rename to drivers/gpu/drm/display/drm_bridge_connector.c
diff --git a/drivers/gpu/drm/imx/dcss/Kconfig b/drivers/gpu/drm/imx/dcss/Kconfig
index 3ffc061d392b..59e3b6a1dff0 100644
--- a/drivers/gpu/drm/imx/dcss/Kconfig
+++ b/drivers/gpu/drm/imx/dcss/Kconfig
@@ -2,6 +2,8 @@ config DRM_IMX_DCSS
tristate "i.MX8MQ DCSS"
select IMX_IRQSTEER
select DRM_KMS_HELPER
+ select DRM_DISPLAY_HELPER
+ select DRM_BRIDGE_CONNECTOR
select DRM_GEM_DMA_HELPER
select VIDEOMODE_HELPERS
depends on DRM && ARCH_MXC && ARM64
diff --git a/drivers/gpu/drm/imx/lcdc/Kconfig b/drivers/gpu/drm/imx/lcdc/Kconfig
index 7e57922bbd9d..9c28bb0f4662 100644
--- a/drivers/gpu/drm/imx/lcdc/Kconfig
+++ b/drivers/gpu/drm/imx/lcdc/Kconfig
@@ -3,5 +3,7 @@ config DRM_IMX_LCDC
depends on DRM && (ARCH_MXC || COMPILE_TEST)
select DRM_GEM_DMA_HELPER
select DRM_KMS_HELPER
+ select DRM_DISPLAY_HELPER
+ select DRM_BRIDGE_CONNECTOR
help
Found on i.MX1, i.MX21, i.MX25 and i.MX27.
diff --git a/drivers/gpu/drm/ingenic/Kconfig b/drivers/gpu/drm/ingenic/Kconfig
index 3db117c5edd9..8cd7b750dffe 100644
--- a/drivers/gpu/drm/ingenic/Kconfig
+++ b/drivers/gpu/drm/ing
[PATCH v3 1/4] drm/drm_property: require DRM_MODE_PROP_IMMUTABLE for single-value props
Document that DRM_MODE_PROP_IMMUTABLE must be set for the properties
that are immutable by definition - e.g. ranges with min == max or enums
with a single value. This matches the behaviour of the IGT tests, see
kms_properties.c / validate_range_prop(), validate_enum_prop(),
validate_bitmask_prop().
Signed-off-by: Dmitry Baryshkov
---
include/drm/drm_property.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/drm/drm_property.h b/include/drm/drm_property.h
index 082f29156b3e..7d0f793f50ca 100644
--- a/include/drm/drm_property.h
+++ b/include/drm/drm_property.h
@@ -162,6 +162,9 @@ struct drm_property {
* userspace, e.g. the EDID, or the connector path property on DP
* MST sinks. Kernel can update the value of an immutable property
* by calling drm_object_property_set_value().
+* This flag should be set for all properties that have only a
+* single value (e.g. min == max or if enum has only a single
+* value).
*/
uint32_t flags;
--
2.39.2
[PATCH v3 0/4] drm: fix two issues related to HDMI Connector implementation
Running IGT tests on Qualcomm Dragonboard820c uncovered two issues with
the HDMI Connector implementation and with its integration into the
drm_bridge_connector. Fix those issues.
Note, I'm not fully satisfied with the drm_bridge_connector move. Maybe
it's better to add drm_bridge_funcs::connector_reset() and call it from
__drm_atomic_helper_connector_reset().
Signed-off-by: Dmitry Baryshkov
---
Changes in v3:
- Document the DRM_MODE_PROP_IMMUTABLE requirements currently exposed
only via IGT tests (Maxime).
- Move drm_bridge_connector to drm_display_helper.
- Link to v2:
https://lore.kernel.org/r/20240623-drm-bridge-connector-fix-hdmi-reset-v2-0-8590d4491...@linaro.org
Changes in v2:
- Actually pass the flags to drm_property_create_range().
- Link to v1:
https://lore.kernel.org/r/20240623-drm-bridge-connector-fix-hdmi-reset-v1-0-41e9894dc...@linaro.org
---
Dmitry Baryshkov (4):
drm/drm_property: require DRM_MODE_PROP_IMMUTABLE for single-value props
drm/connector: automatically set immutable flag for max_bpc property
drm/bridge-connector: move to DRM_DISPLAY_HELPER module
drm/bridge-connector: reset the HDMI connector state
drivers/gpu/drm/Makefile | 1 -
drivers/gpu/drm/bridge/Kconfig | 1 +
drivers/gpu/drm/display/Kconfig | 7 +++
drivers/gpu/drm/display/Makefile | 2 ++
drivers/gpu/drm/{ => display}/drm_bridge_connector.c | 13 -
drivers/gpu/drm/drm_connector.c | 7 ++-
drivers/gpu/drm/imx/dcss/Kconfig | 2 ++
drivers/gpu/drm/imx/lcdc/Kconfig | 2 ++
drivers/gpu/drm/ingenic/Kconfig | 2 ++
drivers/gpu/drm/kmb/Kconfig | 2 ++
drivers/gpu/drm/mediatek/Kconfig | 2 ++
drivers/gpu/drm/meson/Kconfig| 2 ++
drivers/gpu/drm/msm/Kconfig | 1 +
drivers/gpu/drm/omapdrm/Kconfig | 2 ++
drivers/gpu/drm/renesas/rcar-du/Kconfig | 2 ++
drivers/gpu/drm/renesas/rz-du/Kconfig| 2 ++
drivers/gpu/drm/renesas/shmobile/Kconfig | 2 ++
drivers/gpu/drm/rockchip/Kconfig | 4
drivers/gpu/drm/tegra/Kconfig| 1 +
drivers/gpu/drm/tidss/Kconfig| 2 ++
drivers/gpu/drm/xlnx/Kconfig | 1 +
include/drm/drm_property.h | 3 +++
22 files changed, 60 insertions(+), 3 deletions(-)
---
base-commit: 82e4255305c554b0bb18b7ccf2db86041b4c8b6e
change-id: 20240623-drm-bridge-connector-fix-hdmi-reset-0ce86af053aa
Best regards,
--
Dmitry Baryshkov
[PATCH v3 2/4] drm/connector: automatically set immutable flag for max_bpc property
With the introduction of the HDMI Connector framework the driver might
end up creating the max_bpc property with min = max = 8. IGT insists
that such properties carry the 'immutable' flag. Automatically set the
flag if the driver asks for the max_bpc property with min == max.
Fixes: aadb3e16b8f3 ("drm/connector: hdmi: Add output BPC to the connector
state")
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/drm_connector.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index ab6ab7ff7ea8..33847fd63628 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2610,7 +2610,12 @@ int drm_connector_attach_max_bpc_property(struct
drm_connector *connector,
prop = connector->max_bpc_property;
if (!prop) {
- prop = drm_property_create_range(dev, 0, "max bpc", min, max);
+ u32 flags = 0;
+
+ if (min == max)
+ flags |= DRM_MODE_PROP_IMMUTABLE;
+
+ prop = drm_property_create_range(dev, flags, "max bpc", min,
max);
if (!prop)
return -ENOMEM;
--
2.39.2
Re: [PATCH] drm/msm/a6xx: Add missing __always_unused
On Mon, Jul 01, 2024 at 02:23:29PM GMT, Rob Clark wrote: > From: Rob Clark > > The __build_asserts() function only exists to have a place to put > build-time asserts. > > Reported-by: kernel test robot > Closes: > https://lore.kernel.org/oe-kbuild-all/202407010401.rfunrbsx-...@intel.com/ > Signed-off-by: Rob Clark > --- > drivers/gpu/drm/msm/adreno/a6xx_catalog.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH 3/5] drm/msm/iommu: introduce msm_iommu_disp_new() for msm_kms
On Fri, Jun 28, 2024 at 02:48:45PM GMT, Abhinav Kumar wrote: > Introduce a new API msm_iommu_disp_new() for display use-cases. > > Signed-off-by: Abhinav Kumar > --- > drivers/gpu/drm/msm/msm_iommu.c | 26 ++ > drivers/gpu/drm/msm/msm_mmu.h | 1 + > 2 files changed, 27 insertions(+) > Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH] drm/msm/dpu: check ubwc support before adding compressed formats
On Fri, Jun 28, 2024 at 04:39:27PM GMT, Abhinav Kumar wrote: > On QCM2290 chipset DPU does not support UBWC. > > Add a dpu cap to indicate this and do not expose compressed formats > in this case. > > changes since RFC: > - use ubwc enc and dec version of mdss_data instead of catalog > to decide if ubwc is supported > > Signed-off-by: Abhinav Kumar > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) > Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
Re: [PATCH 5/5] drm/msm/dpu: rate limit snapshot capture for mmu faults
On Fri, Jun 28, 2024 at 02:48:47PM GMT, Abhinav Kumar wrote:
> There is no recovery mechanism in place yet to recover from mmu
> faults for DPU. We can only prevent the faults by making sure there
> is no misconfiguration.
>
> Rate-limit the snapshot capture for mmu faults to once per
> msm_kms_init_aspace() as that should be sufficient to capture
> the snapshot for debugging otherwise there will be a lot of
> dpu snapshots getting captured for the same fault which is
> redundant and also might affect capturing even one snapshot
> accurately.
Please squash this into the first patch. There is no need to add code
with a known defficiency.
Also, is there a reason why you haven't used ?
>
> Signed-off-by: Abhinav Kumar
> ---
> drivers/gpu/drm/msm/msm_kms.c | 6 +-
> drivers/gpu/drm/msm/msm_kms.h | 3 +++
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_kms.c b/drivers/gpu/drm/msm/msm_kms.c
> index d5d3117259cf..90a333920c01 100644
> --- a/drivers/gpu/drm/msm/msm_kms.c
> +++ b/drivers/gpu/drm/msm/msm_kms.c
> @@ -168,7 +168,10 @@ static int msm_kms_fault_handler(void *arg, unsigned
> long iova, int flags, void
> {
> struct msm_kms *kms = arg;
>
> - msm_disp_snapshot_state(kms->dev);
> + if (!kms->fault_snapshot_capture) {
> + msm_disp_snapshot_state(kms->dev);
> + kms->fault_snapshot_capture++;
When is it decremented?
> + }
>
> return -ENOSYS;
> }
> @@ -208,6 +211,7 @@ struct msm_gem_address_space *msm_kms_init_aspace(struct
> drm_device *dev)
> mmu->funcs->destroy(mmu);
> }
>
> + kms->fault_snapshot_capture = 0;
> msm_mmu_set_fault_handler(aspace->mmu, kms, msm_kms_fault_handler);
>
> return aspace;
> diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
> index 1e0c54de3716..240b39e60828 100644
> --- a/drivers/gpu/drm/msm/msm_kms.h
> +++ b/drivers/gpu/drm/msm/msm_kms.h
> @@ -134,6 +134,9 @@ struct msm_kms {
> int irq;
> bool irq_requested;
>
> + /* rate limit the snapshot capture to once per attach */
> + int fault_snapshot_capture;
> +
> /* mapper-id used to request GEM buffer mapped for scanout: */
> struct msm_gem_address_space *aspace;
>
> --
> 2.44.0
>
--
With best wishes
Dmitry
Re: [PATCH 0/5] drm/msm: add a display mmu fault handler
On Fri, Jun 28, 2024 at 02:48:42PM GMT, Abhinav Kumar wrote: > To debug display mmu faults, this series introduces a display fault > handler similar to the gpu one. > > This series has been tested on sc7280 chromebook by using triggering > a smmu fault by forcing an incorrect stride on the planes. > > changes since RFC: RFC was v1, so technically this is v2. > - move msm_mmu_set_fault_handler() to msm_kms_init_aspace > - make msm_kms_fault_handler return -ENOSYS > - use msm_disp_snapshot_state() instead of > msm_disp_snapshot_state_sync() > because smmu fault handler should not sleep > - add a rate limiter for the snapshot to avoid spam > > Abhinav Kumar (5): > drm/msm: register a fault handler for display mmu faults > drm/msm/iommu: rename msm_fault_handler to msm_gpu_fault_handler > drm/msm/iommu: introduce msm_iommu_disp_new() for msm_kms > drm/msm: switch msm_kms to use msm_iommu_disp_new() > drm/msm/dpu: rate limit snapshot capture for mmu faults > > drivers/gpu/drm/msm/msm_iommu.c | 32 +--- > drivers/gpu/drm/msm/msm_kms.c | 19 ++- > drivers/gpu/drm/msm/msm_kms.h | 3 +++ > drivers/gpu/drm/msm/msm_mmu.h | 1 + > 4 files changed, 51 insertions(+), 4 deletions(-) > > -- > 2.44.0 > -- With best wishes Dmitry
Re: [RFC PATCH] drm/msm/dpu: check ubwc support before adding compressed formats
On Fri, 28 Jun 2024 at 08:44, Abhinav Kumar wrote:
>
>
>
> On 6/27/2024 4:22 PM, Dmitry Baryshkov wrote:
> > On Fri, 28 Jun 2024 at 00:21, Abhinav Kumar
> > wrote:
> >>
> >>
> >>
> >> On 6/27/2024 2:13 PM, Rob Clark wrote:
> >>> On Thu, Jun 27, 2024 at 1:53 PM Abhinav Kumar
> >>> wrote:
> >>>>
> >>>> On QCM2290 chipset DPU does not support UBWC.
> >>>>
> >>>> Add a dpu cap to indicate this and do not expose compressed formats
> >>>> in this case.
> >>>>
> >>>> Signed-off-by: Abhinav Kumar
> >>>> ---
> >>>>drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h | 1 +
> >>>>drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++
> >>>>drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 5 -
> >>>>3 files changed, 7 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h
> >>>> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h
> >>>> index 3cbb2fe8aba2..6671f798bacc 100644
> >>>> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h
> >>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h
> >>>> @@ -12,6 +12,7 @@ static const struct dpu_caps qcm2290_dpu_caps = {
> >>>> .max_mixer_blendstages = 0x4,
> >>>> .has_dim_layer = true,
> >>>> .has_idle_pc = true,
> >>>> + .has_no_ubwc = true,
> >>>> .max_linewidth = 2160,
> >>>> .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE,
> >>>>};
> >>>> 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 af2ead1c4886..676d0a283922 100644
> >>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> >>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> >>>> @@ -342,6 +342,7 @@ struct dpu_rotation_cfg {
> >>>> * @has_dim_layer dim layer feature status
> >>>> * @has_idle_pcindicate if idle power collapse feature is
> >>>> supported
> >>>> * @has_3d_merge indicate if 3D merge is supported
> >>>> + * @has_no_ubwcindicate if UBWC is supported
> >>>> * @max_linewidth max linewidth for sspp
> >>>> * @pixel_ram_size size of latency hiding and de-tiling buffer in
> >>>> bytes
> >>>> * @max_hdeci_exp max horizontal decimation supported (max is
> >>>> 2^value)
> >>>> @@ -354,6 +355,7 @@ struct dpu_caps {
> >>>> bool has_dim_layer;
> >>>> bool has_idle_pc;
> >>>> bool has_3d_merge;
> >>>> + bool has_no_ubwc;
> >>>
> >>> has_no_ubwc sounds kinda awkward compared to has_ubwc. But I guess
> >>> you wanted to avoid all that churn..
> >>>
> >>
> >> Yes I wanted to avoid modifying all the catalogs.
> >>
> >>> How about instead, if msm_mdss_data::ubwc_{enc,dec}_version are zero,
> >>> then we know there is no ubwc support in the display.
> >>>
> >>
> >> hmm ... should work I can post a v2 with this and avoid touching
> >> the catalog altogether.
> >
> > Yes, this sounds much better.
> >
>
> Ok, does this qualify for a Fixes tag too? Because exposing ubwc formats
> on non-ubwc supported chipsets seems like a bug.
I think it does.
For example, I've also added Fixes: to the patch disabling YUV on qcm2290.
--
With best wishes
Dmitry
[PATCH 2/2] clk: qcom: gpucc-*: use qcom_cc_map_norequest
On most of the Qualcomm platforms GPU clock controller registers are
located inside the GMU's register space. By using qcom_cc_map() gpucc
drivers mark the region as used, prevening GMU driver from claiming the
bigger region.
Make affected GPU clock controller drivers use qcom_cc_map_norequest(),
allowing GMU driver to use devm_ioremap_resource().
Signed-off-by: Dmitry Baryshkov
---
drivers/clk/qcom/gpucc-qcm2290.c | 2 +-
drivers/clk/qcom/gpucc-sa8775p.c | 2 +-
drivers/clk/qcom/gpucc-sc7180.c | 2 +-
drivers/clk/qcom/gpucc-sc7280.c | 2 +-
drivers/clk/qcom/gpucc-sc8280xp.c | 2 +-
drivers/clk/qcom/gpucc-sdm845.c | 2 +-
drivers/clk/qcom/gpucc-sm6115.c | 2 +-
drivers/clk/qcom/gpucc-sm6125.c | 2 +-
drivers/clk/qcom/gpucc-sm6350.c | 2 +-
drivers/clk/qcom/gpucc-sm6375.c | 2 +-
drivers/clk/qcom/gpucc-sm8150.c | 2 +-
drivers/clk/qcom/gpucc-sm8250.c | 2 +-
drivers/clk/qcom/gpucc-sm8350.c | 2 +-
drivers/clk/qcom/gpucc-sm8450.c | 2 +-
drivers/clk/qcom/gpucc-sm8550.c | 2 +-
drivers/clk/qcom/gpucc-sm8650.c | 2 +-
drivers/clk/qcom/gpucc-x1e80100.c | 2 +-
17 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/clk/qcom/gpucc-qcm2290.c b/drivers/clk/qcom/gpucc-qcm2290.c
index dc369dff882e..2a886b3d6ab4 100644
--- a/drivers/clk/qcom/gpucc-qcm2290.c
+++ b/drivers/clk/qcom/gpucc-qcm2290.c
@@ -372,7 +372,7 @@ static int gpu_cc_qcm2290_probe(struct platform_device
*pdev)
struct regmap *regmap;
int ret;
- regmap = qcom_cc_map(pdev, _cc_qcm2290_desc);
+ regmap = qcom_cc_map_norequest(pdev, _cc_qcm2290_desc);
if (IS_ERR(regmap))
return PTR_ERR(regmap);
diff --git a/drivers/clk/qcom/gpucc-sa8775p.c b/drivers/clk/qcom/gpucc-sa8775p.c
index f8a8ac343d70..312b45e6fc29 100644
--- a/drivers/clk/qcom/gpucc-sa8775p.c
+++ b/drivers/clk/qcom/gpucc-sa8775p.c
@@ -592,7 +592,7 @@ static int gpu_cc_sa8775p_probe(struct platform_device
*pdev)
{
struct regmap *regmap;
- regmap = qcom_cc_map(pdev, _cc_sa8775p_desc);
+ regmap = qcom_cc_map_norequest(pdev, _cc_sa8775p_desc);
if (IS_ERR(regmap))
return PTR_ERR(regmap);
diff --git a/drivers/clk/qcom/gpucc-sc7180.c b/drivers/clk/qcom/gpucc-sc7180.c
index 08f3983d016f..03480a2fa78c 100644
--- a/drivers/clk/qcom/gpucc-sc7180.c
+++ b/drivers/clk/qcom/gpucc-sc7180.c
@@ -220,7 +220,7 @@ static int gpu_cc_sc7180_probe(struct platform_device *pdev)
struct alpha_pll_config gpu_cc_pll_config = {};
unsigned int value, mask;
- regmap = qcom_cc_map(pdev, _cc_sc7180_desc);
+ regmap = qcom_cc_map_norequest(pdev, _cc_sc7180_desc);
if (IS_ERR(regmap))
return PTR_ERR(regmap);
diff --git a/drivers/clk/qcom/gpucc-sc7280.c b/drivers/clk/qcom/gpucc-sc7280.c
index bd699a624517..86f89fbb4aec 100644
--- a/drivers/clk/qcom/gpucc-sc7280.c
+++ b/drivers/clk/qcom/gpucc-sc7280.c
@@ -458,7 +458,7 @@ static int gpu_cc_sc7280_probe(struct platform_device *pdev)
{
struct regmap *regmap;
- regmap = qcom_cc_map(pdev, _cc_sc7280_desc);
+ regmap = qcom_cc_map_norequest(pdev, _cc_sc7280_desc);
if (IS_ERR(regmap))
return PTR_ERR(regmap);
diff --git a/drivers/clk/qcom/gpucc-sc8280xp.c
b/drivers/clk/qcom/gpucc-sc8280xp.c
index c96be61e3f47..519940dc99eb 100644
--- a/drivers/clk/qcom/gpucc-sc8280xp.c
+++ b/drivers/clk/qcom/gpucc-sc8280xp.c
@@ -436,7 +436,7 @@ static int gpu_cc_sc8280xp_probe(struct platform_device
*pdev)
if (ret)
return ret;
- regmap = qcom_cc_map(pdev, _cc_sc8280xp_desc);
+ regmap = qcom_cc_map_norequest(pdev, _cc_sc8280xp_desc);
if (IS_ERR(regmap)) {
pm_runtime_put(>dev);
return PTR_ERR(regmap);
diff --git a/drivers/clk/qcom/gpucc-sdm845.c b/drivers/clk/qcom/gpucc-sdm845.c
index ef26690cf504..b78f8b632601 100644
--- a/drivers/clk/qcom/gpucc-sdm845.c
+++ b/drivers/clk/qcom/gpucc-sdm845.c
@@ -177,7 +177,7 @@ static int gpu_cc_sdm845_probe(struct platform_device *pdev)
struct regmap *regmap;
unsigned int value, mask;
- regmap = qcom_cc_map(pdev, _cc_sdm845_desc);
+ regmap = qcom_cc_map_norequest(pdev, _cc_sdm845_desc);
if (IS_ERR(regmap))
return PTR_ERR(regmap);
diff --git a/drivers/clk/qcom/gpucc-sm6115.c b/drivers/clk/qcom/gpucc-sm6115.c
index d43c86cf73a5..ab3e33fbe401 100644
--- a/drivers/clk/qcom/gpucc-sm6115.c
+++ b/drivers/clk/qcom/gpucc-sm6115.c
@@ -474,7 +474,7 @@ static int gpu_cc_sm6115_probe(struct platform_device *pdev)
{
struct regmap *regmap;
- regmap = qcom_cc_map(pdev, _cc_sm6115_desc);
+ regmap = qcom_cc_map_norequest(pdev, _cc_sm6115_desc);
if (IS_ERR(regmap))
return PTR_ERR(regmap);
diff --git a/drivers/clk/qcom/gpucc-sm6125.c b/drivers/clk/qcom/gpucc-sm6125.c
index ed6a6e505801..14dc75b3771a 100644
--- a/drivers/clk/qcom/gpucc-sm6125.c
+++ b/driv
[PATCH 0/2] clk: qcom: gpucc-*: stop requesting register resource
Testing of [1] pointed out that on all modern Qualcomm platforms GPU clock controller shares regiser space with the GMU. All gpucc drivers use (internally) devm_platform_ioremap_resource(), preventing the GPU driver from using devm_ioremap_resource() on its own. As GMU register space includes gpucc's one, make gpucc drivers use non-requesting helper, allowing GPU to take over the bigger memory region. [1] https://patchwork.freedesktop.org/series/134401/ Signed-off-by: Dmitry Baryshkov --- Dmitry Baryshkov (2): clk: qocm: add qcom_cc_map_norequest clk: qcom: gpucc-*: use qcom_cc_map_norequest drivers/clk/qcom/common.c | 20 drivers/clk/qcom/common.h | 2 ++ drivers/clk/qcom/gpucc-qcm2290.c | 2 +- drivers/clk/qcom/gpucc-sa8775p.c | 2 +- drivers/clk/qcom/gpucc-sc7180.c | 2 +- drivers/clk/qcom/gpucc-sc7280.c | 2 +- drivers/clk/qcom/gpucc-sc8280xp.c | 2 +- drivers/clk/qcom/gpucc-sdm845.c | 2 +- drivers/clk/qcom/gpucc-sm6115.c | 2 +- drivers/clk/qcom/gpucc-sm6125.c | 2 +- drivers/clk/qcom/gpucc-sm6350.c | 2 +- drivers/clk/qcom/gpucc-sm6375.c | 2 +- drivers/clk/qcom/gpucc-sm8150.c | 2 +- drivers/clk/qcom/gpucc-sm8250.c | 2 +- drivers/clk/qcom/gpucc-sm8350.c | 2 +- drivers/clk/qcom/gpucc-sm8450.c | 2 +- drivers/clk/qcom/gpucc-sm8550.c | 2 +- drivers/clk/qcom/gpucc-sm8650.c | 2 +- drivers/clk/qcom/gpucc-x1e80100.c | 2 +- 19 files changed, 39 insertions(+), 17 deletions(-) --- base-commit: 5d98d5e70f505b7278336de493eba94cde5526b3 change-id: 20240627-gpucc-no-request-cb6b5f72e8da Best regards, -- Dmitry Baryshkov
[PATCH 1/2] clk: qocm: add qcom_cc_map_norequest
The GPU clock controllers use memory region that is a part of the GMU's
memory region. Add qcom_cc_map_norequest() to be used by GPUCC, so that
GPU driver can use devm_ioremap_resource for GMU resources.
Signed-off-by: Dmitry Baryshkov
---
drivers/clk/qcom/common.c | 20
drivers/clk/qcom/common.h | 2 ++
2 files changed, 22 insertions(+)
diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
index c92e10c60322..dcc73bc22606 100644
--- a/drivers/clk/qcom/common.c
+++ b/drivers/clk/qcom/common.c
@@ -113,6 +113,26 @@ qcom_cc_map(struct platform_device *pdev, const struct
qcom_cc_desc *desc)
}
EXPORT_SYMBOL_GPL(qcom_cc_map);
+/* gpucc shares memory region with GMU, don't request the region */
+struct regmap *
+qcom_cc_map_norequest(struct platform_device *pdev, const struct qcom_cc_desc
*desc)
+{
+ struct device *dev = >dev;
+ struct resource *r;
+ void __iomem *base;
+
+ r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!r)
+ return ERR_PTR(dev_err_probe(dev, -EINVAL, "resource not
found\n"));
+
+ base = devm_ioremap(dev, r->start, resource_size(r));
+ if (IS_ERR(base))
+ return ERR_CAST(base);
+
+ return devm_regmap_init_mmio(dev, base, desc->config);
+}
+EXPORT_SYMBOL_GPL(qcom_cc_map_norequest);
+
void
qcom_pll_set_fsm_mode(struct regmap *map, u32 reg, u8 bias_count, u8
lock_count)
{
diff --git a/drivers/clk/qcom/common.h b/drivers/clk/qcom/common.h
index d048bdeeba10..6cab7805a92c 100644
--- a/drivers/clk/qcom/common.h
+++ b/drivers/clk/qcom/common.h
@@ -60,6 +60,8 @@ extern int qcom_cc_register_sleep_clk(struct device *dev);
extern struct regmap *qcom_cc_map(struct platform_device *pdev,
const struct qcom_cc_desc *desc);
+extern struct regmap *qcom_cc_map_norequest(struct platform_device *pdev,
+ const struct qcom_cc_desc *desc);
extern int qcom_cc_really_probe(struct device *dev,
const struct qcom_cc_desc *desc,
struct regmap *regmap);
--
2.39.2
Re: [RFC PATCH] drm/msm/dpu: check ubwc support before adding compressed formats
On Fri, 28 Jun 2024 at 00:21, Abhinav Kumar wrote:
>
>
>
> On 6/27/2024 2:13 PM, Rob Clark wrote:
> > On Thu, Jun 27, 2024 at 1:53 PM Abhinav Kumar
> > wrote:
> >>
> >> On QCM2290 chipset DPU does not support UBWC.
> >>
> >> Add a dpu cap to indicate this and do not expose compressed formats
> >> in this case.
> >>
> >> Signed-off-by: Abhinav Kumar
> >> ---
> >> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h | 1 +
> >> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++
> >> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 5 -
> >> 3 files changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h
> >> b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h
> >> index 3cbb2fe8aba2..6671f798bacc 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_5_qcm2290.h
> >> @@ -12,6 +12,7 @@ static const struct dpu_caps qcm2290_dpu_caps = {
> >> .max_mixer_blendstages = 0x4,
> >> .has_dim_layer = true,
> >> .has_idle_pc = true,
> >> + .has_no_ubwc = true,
> >> .max_linewidth = 2160,
> >> .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE,
> >> };
> >> 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 af2ead1c4886..676d0a283922 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> >> @@ -342,6 +342,7 @@ struct dpu_rotation_cfg {
> >>* @has_dim_layer dim layer feature status
> >>* @has_idle_pcindicate if idle power collapse feature is
> >> supported
> >>* @has_3d_merge indicate if 3D merge is supported
> >> + * @has_no_ubwcindicate if UBWC is supported
> >>* @max_linewidth max linewidth for sspp
> >>* @pixel_ram_size size of latency hiding and de-tiling buffer in
> >> bytes
> >>* @max_hdeci_exp max horizontal decimation supported (max is
> >> 2^value)
> >> @@ -354,6 +355,7 @@ struct dpu_caps {
> >> bool has_dim_layer;
> >> bool has_idle_pc;
> >> bool has_3d_merge;
> >> + bool has_no_ubwc;
> >
> > has_no_ubwc sounds kinda awkward compared to has_ubwc. But I guess
> > you wanted to avoid all that churn..
> >
>
> Yes I wanted to avoid modifying all the catalogs.
>
> > How about instead, if msm_mdss_data::ubwc_{enc,dec}_version are zero,
> > then we know there is no ubwc support in the display.
> >
>
> hmm ... should work I can post a v2 with this and avoid touching
> the catalog altogether.
Yes, this sounds much better.
--
With best wishes
Dmitry
Re: [PATCH] drm/msm/a6xx: request memory region
On Thu, Jun 27, 2024 at 03:22:18AM GMT, Akhil P Oommen wrote: > << snip >> > > > > > > > > @@ -1503,7 +1497,7 @@ static void __iomem > > > > > > > *a6xx_gmu_get_mmio(struct platform_device *pdev, > > > > > > > return ERR_PTR(-EINVAL); > > > > > > > } > > > > > > > > > > > > > > - ret = ioremap(res->start, resource_size(res)); > > > > > > > + ret = devm_ioremap_resource(>dev, res); > > > > > > > > > > > > So, this doesn't actually work, failing in > > > > > > __request_region_locked(), > > > > > > because the gmu region partially overlaps with the gpucc region > > > > > > (which > > > > > > is busy). I think this is intentional, since gmu is controlling the > > > > > > gpu clocks, etc. In particular REG_A6XX_GPU_CC_GX_GDSCR is in this > > > > > > overlapping region. Maybe Akhil knows more about GMU. > > > > > > > > > > We don't really need to map gpucc region from driver on behalf of gmu. > > > > > Since we don't access any gpucc register from drm-msm driver, we can > > > > > update the range size to correct this. But due to backward > > > > > compatibility > > > > > requirement with older dt, can we still enable region locking? I > > > > > prefer > > > > > it if that is possible. > > > > > > > > Actually, when I reduced the region size to not overlap with gpucc, > > > > the region is smaller than REG_A6XX_GPU_CC_GX_GDSCR * 4. > > > > > > > > So I guess that register is actually part of gpucc? > > > > > > Yes. It has *GPU_CC* in its name. :P > > > > > > I just saw that we program this register on legacy a6xx targets to > > > ensure retention is really ON before collapsing gdsc. So we can't > > > avoid mapping gpucc region in legacy a6xx GPUs. That is unfortunate! > > > > I guess we could still use devm_ioremap().. idk if there is a better > > way to solve this > > Can we do it without breaking backward compatibility with dt? I think a proper way would be to use devm_ioremap in the gpucc driver, then the GPU driver can use devm_platform_ioremap_resource(). I'll take a look at sketching the gpucc patches in one of the next few days. > > -Akhil > > > > > BR, > > -R > > > > > -Akhil. > > > > > > > > > > > BR, > > > > -R -- With best wishes Dmitry
[PATCH v5 10/12] drm/msm/dpu: add support for virtual planes
Only several SSPP blocks support such features as YUV output or scaling,
thus different DRM planes have different features. Properly utilizing
all planes requires the attention of the compositor, who should
prefer simpler planes to YUV-supporting ones. Otherwise it is very easy
to end up in a situation when all featureful planes are already
allocated for simple windows, leaving no spare plane for YUV playback.
To solve this problem make all planes virtual. Each plane is registered
as if it supports all possible features, but then at the runtime during
the atomic_check phase the driver selects backing SSPP block for each
plane.
As the planes are attached to the CRTC and not the the encoder, the
SSPP blocks are also allocated per CRTC ID (all other resources are
currently allocated per encoder ID). This also matches the hardware
requirement, where both rectangles of a single SSPP can only be used
with the LM pair.
Note, this does not provide support for using two different SSPP blocks
for a single plane or using two rectangles of an SSPP to drive two
planes. Each plane still gets its own SSPP and can utilize either a solo
rectangle or both multirect rectangles depending on the resolution.
Note #2: By default support for virtual planes is turned off and the
driver still uses old code path with preallocated SSPP block for each
plane. To enable virtual planes, pass 'msm.dpu_use_virtual_planes=1'
kernel parameter.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 50 +++
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 10 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 4 +
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 236 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 16 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c| 77 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h| 27
7 files changed, 391 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 9f2164782844..c438cf834320 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1168,6 +1168,49 @@ static bool dpu_crtc_needs_dirtyfb(struct drm_crtc_state
*cstate)
return false;
}
+static int dpu_crtc_reassign_planes(struct drm_crtc *crtc, struct
drm_crtc_state *crtc_state)
+{
+ int total_planes = crtc->dev->mode_config.num_total_plane;
+ struct drm_atomic_state *state = crtc_state->state;
+ struct dpu_global_state *global_state;
+ struct drm_plane_state **states;
+ struct drm_plane *plane;
+ int ret;
+
+ global_state = dpu_kms_get_global_state(crtc_state->state);
+ if (IS_ERR(global_state))
+ return PTR_ERR(global_state);
+
+ dpu_rm_release_all_sspp(global_state, crtc);
+
+ if (!crtc_state->enable)
+ return 0;
+
+ states = kcalloc(total_planes, sizeof(*states), GFP_KERNEL);
+ if (!states)
+ return -ENOMEM;
+
+ drm_atomic_crtc_state_for_each_plane(plane, crtc_state) {
+ struct drm_plane_state *plane_state =
+ drm_atomic_get_plane_state(state, plane);
+
+ if (IS_ERR(plane_state)) {
+ ret = PTR_ERR(plane_state);
+ goto done;
+ }
+
+ states[plane_state->normalized_zpos] = plane_state;
+ }
+
+ ret = dpu_assign_plane_resources(global_state, state, crtc, states,
total_planes);
+
+done:
+ kfree(states);
+ return ret;
+
+ return 0;
+}
+
static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
struct drm_atomic_state *state)
{
@@ -1183,6 +1226,13 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
bool needs_dirtyfb = dpu_crtc_needs_dirtyfb(crtc_state);
+ if (dpu_use_virtual_planes &&
+ (crtc_state->planes_changed || crtc_state->zpos_changed)) {
+ rc = dpu_crtc_reassign_planes(crtc, crtc_state);
+ if (rc < 0)
+ return rc;
+ }
+
if (!crtc_state->enable ||
!drm_atomic_crtc_effectively_active(crtc_state)) {
DRM_DEBUG_ATOMIC("crtc%d -> enable %d, active %d, skip
atomic_check\n",
crtc->base.id, crtc_state->enable,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index d1e2143110f2..449fdc0d33dd 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -51,6 +51,9 @@
#define DPU_DEBUGFS_DIR "msm_dpu"
#define DPU_DEBUGFS_HWMASKNAME "hw_log_mask"
+bool dpu_use_virtual_planes = false;
+module_param(dpu_use_virtual_planes, bool, 0);
+
static int dpu_kms_hw_init(struct msm_kms *kms);
static void _dpu_kms_mmu_destroy(struct dpu_kms *dpu_kms);
[PATCH v5 12/12] drm/msm/dpu: include SSPP allocation state into the dumped state
Make dpu_rm_print_state() also output the SSPP allocation state. Reviewed-by: Abhinav Kumar Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c index dbf1f7229e45..3e3b6b8daa5c 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c @@ -890,4 +890,11 @@ void dpu_rm_print_state(struct drm_printer *p, dpu_rm_print_state_helper(p, rm->cdm_blk, global_state->cdm_to_enc_id); drm_puts(p, "\n"); + + drm_puts(p, "\tsspp="); + /* skip SSPP_NONE and start from the next index */ + for (i = SSPP_NONE + 1; i < ARRAY_SIZE(global_state->sspp_to_crtc_id); i++) + dpu_rm_print_state_helper(p, rm->hw_sspp[i] ? >hw_sspp[i]->base : NULL, + global_state->sspp_to_crtc_id[i]); + drm_puts(p, "\n"); } -- 2.39.2
[PATCH v5 08/12] drm/msm/dpu: split dpu_plane_atomic_check()
Split dpu_plane_atomic_check() function into two pieces:
dpu_plane_atomic_check_nopipe() performing generic checks on the pstate,
without touching the associated pipe,
and
dpu_plane_atomic_check_pipes(), which takes into account used pipes.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 178 +++---
1 file changed, 112 insertions(+), 66 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 115c1bd77bdd..9b9fe28052ad 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -788,49 +788,22 @@ static int dpu_plane_atomic_check_pipe(struct dpu_plane
*pdpu,
#define MAX_UPSCALE_RATIO 20
#define MAX_DOWNSCALE_RATIO4
-static int dpu_plane_atomic_check(struct drm_plane *plane,
- struct drm_atomic_state *state)
+static int dpu_plane_atomic_check_nopipe(struct drm_plane *plane,
+struct drm_plane_state
*new_plane_state,
+const struct drm_crtc_state
*crtc_state)
{
- struct drm_plane_state *new_plane_state =
drm_atomic_get_new_plane_state(state,
-
plane);
int ret = 0, min_scale, max_scale;
struct dpu_plane *pdpu = to_dpu_plane(plane);
struct dpu_kms *kms = _dpu_plane_get_kms(>base);
u64 max_mdp_clk_rate = kms->perf.max_core_clk_rate;
struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state);
- struct dpu_sw_pipe *pipe = >pipe;
- struct dpu_sw_pipe *r_pipe = >r_pipe;
- const struct drm_crtc_state *crtc_state = NULL;
- const struct msm_format *fmt;
struct dpu_sw_pipe_cfg *pipe_cfg = >pipe_cfg;
struct dpu_sw_pipe_cfg *r_pipe_cfg = >r_pipe_cfg;
struct drm_rect fb_rect = { 0 };
uint32_t max_linewidth;
- unsigned int rotation;
- uint32_t supported_rotations;
- const struct dpu_sspp_cfg *pipe_hw_caps;
- const struct dpu_sspp_sub_blks *sblk;
-
- if (new_plane_state->crtc)
- crtc_state = drm_atomic_get_new_crtc_state(state,
-
new_plane_state->crtc);
-
- pipe->sspp = dpu_rm_get_sspp(>rm, pdpu->pipe);
- r_pipe->sspp = NULL;
- if (!pipe->sspp)
- return -EINVAL;
-
- pipe_hw_caps = pipe->sspp->cap;
- sblk = pipe->sspp->cap->sblk;
-
- if (sblk->scaler_blk.len) {
- min_scale = FRAC_16_16(1, MAX_UPSCALE_RATIO);
- max_scale = MAX_DOWNSCALE_RATIO << 16;
- } else {
- min_scale = DRM_PLANE_NO_SCALING;
- max_scale = DRM_PLANE_NO_SCALING;
- }
+ min_scale = FRAC_16_16(1, MAX_UPSCALE_RATIO);
+ max_scale = MAX_DOWNSCALE_RATIO << 16;
ret = drm_atomic_helper_check_plane_state(new_plane_state, crtc_state,
min_scale,
@@ -843,11 +816,6 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
if (!new_plane_state->visible)
return 0;
- pipe->multirect_index = DPU_SSPP_RECT_SOLO;
- pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
- r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
- r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
-
pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos;
if (pstate->stage >= pdpu->catalog->caps->max_mixer_blendstages) {
DPU_ERROR("> %d plane stages assigned\n",
@@ -871,8 +839,6 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
return -E2BIG;
}
- fmt = msm_framebuffer_format(new_plane_state->fb);
-
max_linewidth = pdpu->catalog->caps->max_linewidth;
drm_rect_rotate(_cfg->src_rect,
@@ -881,6 +847,78 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
if ((drm_rect_width(_cfg->src_rect) > max_linewidth) ||
_dpu_plane_calc_clk(_state->adjusted_mode, pipe_cfg) >
max_mdp_clk_rate) {
+ if (drm_rect_width(_cfg->src_rect) > 2 * max_linewidth) {
+ DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT "
line:%u\n",
+ DRM_RECT_ARG(_cfg->src_rect),
max_linewidth);
+ return -E2BIG;
+ }
+
+ *r_pipe_cfg = *pipe_cfg;
+ pipe_cfg->src_rect.x2 = (pipe_cfg->src_rect.x1 +
pipe_cfg->src_rect.x2) >> 1;
+ pipe_cfg->dst_rect.x2 = (pipe_cfg->dst_rect.x1 +
pipe_cfg->dst_rect.x2) >> 1;
+ r_pipe_cfg->src_rect.x1 = pi
[PATCH v5 11/12] drm/msm/dpu: allow using two SSPP blocks for a single plane
Virtual wide planes give high amount of flexibility, but it is not
always enough:
In parallel multirect case only the half of the usual width is supported
for tiled formats. Thus the whole width of two tiled multirect
rectangles can not be greater than max_linewidth, which is not enough
for some platforms/compositors.
Another example is as simple as wide YUV plane. YUV planes can not use
multirect, so currently they are limited to max_linewidth too.
Now that the planes are fully virtualized, add support for allocating
two SSPP blocks to drive a single DRM plane. This fixes both mentioned
cases and allows all planes to go up to 2*max_linewidth (at the cost of
making some of the planes unavailable to the user).
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 161 ++
1 file changed, 117 insertions(+), 44 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index c72c65bf55e1..0d9f9216420b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -19,7 +19,6 @@
#include "msm_drv.h"
#include "dpu_kms.h"
-#include "dpu_formats.h"
#include "dpu_hw_sspp.h"
#include "dpu_hw_util.h"
#include "dpu_trace.h"
@@ -886,6 +885,28 @@ static int dpu_plane_atomic_check_nopipe(struct drm_plane
*plane,
return 0;
}
+static int dpu_plane_is_multirect_parallel_capable(struct dpu_sw_pipe *pipe,
+ struct dpu_sw_pipe_cfg
*pipe_cfg,
+ const struct msm_format *fmt,
+ uint32_t max_linewidth)
+{
+ if (drm_rect_width(_cfg->src_rect) !=
drm_rect_width(_cfg->dst_rect) ||
+ drm_rect_height(_cfg->src_rect) !=
drm_rect_height(_cfg->dst_rect))
+ return false;
+
+ if (pipe_cfg->rotation & DRM_MODE_ROTATE_90)
+ return false;
+
+ if (MSM_FORMAT_IS_YUV(fmt))
+ return false;
+
+ if (MSM_FORMAT_IS_UBWC(fmt) &&
+ drm_rect_width(_cfg->src_rect) > max_linewidth / 2)
+ return false;
+
+ return true;
+}
+
static int dpu_plane_atomic_check_pipes(struct drm_plane *plane,
struct drm_atomic_state *state,
const struct drm_crtc_state *crtc_state)
@@ -899,7 +920,6 @@ static int dpu_plane_atomic_check_pipes(struct drm_plane
*plane,
const struct msm_format *fmt;
struct dpu_sw_pipe_cfg *pipe_cfg = >pipe_cfg;
struct dpu_sw_pipe_cfg *r_pipe_cfg = >r_pipe_cfg;
- uint32_t max_linewidth;
uint32_t supported_rotations;
const struct dpu_sspp_cfg *pipe_hw_caps;
const struct dpu_sspp_sub_blks *sblk;
@@ -921,8 +941,6 @@ static int dpu_plane_atomic_check_pipes(struct drm_plane
*plane,
fmt = msm_framebuffer_format(new_plane_state->fb);
- max_linewidth = pdpu->catalog->caps->max_linewidth;
-
supported_rotations = DRM_MODE_REFLECT_MASK | DRM_MODE_ROTATE_0;
if (pipe_hw_caps->features & BIT(DPU_SSPP_INLINE_ROTATION))
@@ -938,41 +956,6 @@ static int dpu_plane_atomic_check_pipes(struct drm_plane
*plane,
return ret;
if (drm_rect_width(_pipe_cfg->src_rect) != 0) {
- /*
-* In parallel multirect case only the half of the usual width
-* is supported for tiled formats. If we are here, we know that
-* full width is more than max_linewidth, thus each rect is
-* wider than allowed.
-*/
- if (MSM_FORMAT_IS_UBWC(fmt) &&
- drm_rect_width(_cfg->src_rect) > max_linewidth) {
- DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT "
line:%u, tiled format\n",
- DRM_RECT_ARG(_cfg->src_rect),
max_linewidth);
- return -E2BIG;
- }
-
- if (drm_rect_width(_cfg->src_rect) !=
drm_rect_width(_cfg->dst_rect) ||
- drm_rect_height(_cfg->src_rect) !=
drm_rect_height(_cfg->dst_rect) ||
- (!test_bit(DPU_SSPP_SMART_DMA_V1,
>sspp->cap->features) &&
-!test_bit(DPU_SSPP_SMART_DMA_V2,
>sspp->cap->features)) ||
- pipe_cfg->rotation & DRM_MODE_ROTATE_90 ||
- MSM_FORMAT_IS_YUV(fmt)) {
- DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT "
line:%u, can't use split source\n",
- DRM_RECT_ARG(_cfg->src_rect),
max_linewidth);
- return -E2BIG;
-
[PATCH v5 03/12] drm/msm/dpu: take plane rotation into account for wide planes
Take into account the plane rotation and flipping when calculating src
positions for the wide plane parts.
This is not an issue yet, because rotation is only supported for the
UBWC planes and wide UBWC planes are rejected anyway because in parallel
multirect case only the half of the usual width is supported for tiled
formats. However it's better to fix this now rather than stumbling upon
it later.
Fixes: 80e8ae3b38ab ("drm/msm/dpu: add support for wide planes")
Reviewed-by: Abhinav Kumar
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 148bd79bdcef..8f2759d16247 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -862,6 +862,10 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
max_linewidth = pdpu->catalog->caps->max_linewidth;
+ drm_rect_rotate(_cfg->src_rect,
+ new_plane_state->fb->width, new_plane_state->fb->height,
+ new_plane_state->rotation);
+
if ((drm_rect_width(_cfg->src_rect) > max_linewidth) ||
_dpu_plane_calc_clk(_state->adjusted_mode, pipe_cfg) >
max_mdp_clk_rate) {
/*
@@ -911,6 +915,14 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
r_pipe_cfg->dst_rect.x1 = pipe_cfg->dst_rect.x2;
}
+ drm_rect_rotate_inv(_cfg->src_rect,
+ new_plane_state->fb->width,
new_plane_state->fb->height,
+ new_plane_state->rotation);
+ if (r_pipe->sspp)
+ drm_rect_rotate_inv(_pipe_cfg->src_rect,
+ new_plane_state->fb->width,
new_plane_state->fb->height,
+ new_plane_state->rotation);
+
ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg, fmt,
_state->adjusted_mode);
if (ret)
return ret;
--
2.39.2
[PATCH v5 09/12] drm/msm/dpu: move rot90 checking to dpu_plane_atomic_check_pipe()
Move a call to dpu_plane_check_inline_rotation() to the
dpu_plane_atomic_check_pipe() function, so that the rot90 constraints
are checked for both pipes. Also move rotation field from struct
dpu_plane_state to struct dpu_sw_pipe_cfg.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 2 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 55 +++--
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 2 --
3 files changed, 31 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
index 4a910b808687..fc54625ae5d4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
@@ -142,10 +142,12 @@ struct dpu_hw_pixel_ext {
* @src_rect: src ROI, caller takes into account the different operations
* such as decimation, flip etc to program this field
* @dest_rect: destination ROI.
+ * @rotation: simplified drm rotation hint
*/
struct dpu_sw_pipe_cfg {
struct drm_rect src_rect;
struct drm_rect dst_rect;
+ unsigned int rotation;
};
/**
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 9b9fe28052ad..6ad160295341 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -527,8 +527,7 @@ static const struct dpu_csc_cfg *_dpu_plane_get_csc(struct
dpu_sw_pipe *pipe,
static void _dpu_plane_setup_scaler(struct dpu_sw_pipe *pipe,
const struct msm_format *fmt, bool color_fill,
- struct dpu_sw_pipe_cfg *pipe_cfg,
- unsigned int rotation)
+ struct dpu_sw_pipe_cfg *pipe_cfg)
{
struct dpu_hw_sspp *pipe_hw = pipe->sspp;
const struct drm_format_info *info = drm_format_info(fmt->pixel_format);
@@ -551,7 +550,7 @@ static void _dpu_plane_setup_scaler(struct dpu_sw_pipe
*pipe,
dst_height,
_cfg, fmt,
info->hsub, info->vsub,
- rotation);
+ pipe_cfg->rotation);
/* configure pixel extension based on scalar config */
_dpu_plane_setup_pixel_ext(_cfg, _ext,
@@ -603,7 +602,7 @@ static void _dpu_plane_color_fill_pipe(struct
dpu_plane_state *pstate,
if (pipe->sspp->ops.setup_rects)
pipe->sspp->ops.setup_rects(pipe, _cfg);
- _dpu_plane_setup_scaler(pipe, fmt, true, _cfg, pstate->rotation);
+ _dpu_plane_setup_scaler(pipe, fmt, true, _cfg);
}
/**
@@ -704,12 +703,17 @@ static void dpu_plane_cleanup_fb(struct drm_plane *plane,
}
static int dpu_plane_check_inline_rotation(struct dpu_plane *pdpu,
- const struct dpu_sspp_sub_blks
*sblk,
- struct drm_rect src, const
struct msm_format *fmt)
+ struct dpu_sw_pipe *pipe,
+ struct drm_rect src,
+ const struct msm_format *fmt)
{
+ const struct dpu_sspp_sub_blks *sblk = pipe->sspp->cap->sblk;
size_t num_formats;
const u32 *supported_formats;
+ if (!test_bit(DPU_SSPP_INLINE_ROTATION, >sspp->cap->features))
+ return -EINVAL;
+
if (!sblk->rotation_cfg) {
DPU_ERROR("invalid rotation cfg\n");
return -EINVAL;
@@ -739,6 +743,7 @@ static int dpu_plane_atomic_check_pipe(struct dpu_plane
*pdpu,
{
uint32_t min_src_size;
struct dpu_kms *kms = _dpu_plane_get_kms(>base);
+ int ret;
min_src_size = MSM_FORMAT_IS_YUV(fmt) ? 2 : 1;
@@ -776,6 +781,12 @@ static int dpu_plane_atomic_check_pipe(struct dpu_plane
*pdpu,
return -EINVAL;
}
+ if (pipe_cfg->rotation & DRM_MODE_ROTATE_90) {
+ ret = dpu_plane_check_inline_rotation(pdpu, pipe,
pipe_cfg->src_rect, fmt);
+ if (ret)
+ return ret;
+ }
+
/* max clk check */
if (_dpu_plane_calc_clk(mode, pipe_cfg) > kms->perf.max_core_clk_rate) {
DPU_DEBUG_PLANE(pdpu, "plane exceeds max mdp core clk
limits\n");
@@ -889,7 +900,6 @@ static int dpu_plane_atomic_check_pipes(struct drm_plane
*plane,
struct dpu_sw_pipe_cfg *pipe_cfg = >pipe_cfg;
struct dpu_sw_pipe_cfg *r_pipe_cfg = >r_pipe_cfg;
uint32_t max_linewidth;
- unsigned int rotation;
uint32_t supported_rotations;
const struct dpu_sspp_cfg *pipe_hw_caps;
const struct dpu_sspp_sub_blks *sblk;
@@ -913,6 +923,15 @@ static int dpu_plane_atomic_check_pipes(struct drm_plane
*plane,
max_linewidth = pdpu->catalog->caps->max_linewidth;
+ su
[PATCH v5 06/12] drm/msm/dpu: drop virt_formats from SSPP subblock configuration
The virt_formats / virt_num_formats are not used by the current driver
and are not going to be used in future since formats for virtual planes
are handled in a different way, by forbidding unsupported combinations
during atomic_check. Drop those fields now.
Reviewed-by: Abhinav Kumar
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 8
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 4
2 files changed, 12 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index 648c8d0a4c36..a3d29c69bda4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -283,8 +283,6 @@ static const u32 wb2_formats_rgb_yuv[] = {
.base = 0x1a00, .len = 0x100,}, \
.format_list = plane_formats_yuv, \
.num_formats = ARRAY_SIZE(plane_formats_yuv), \
- .virt_format_list = plane_formats, \
- .virt_num_formats = ARRAY_SIZE(plane_formats), \
.rotation_cfg = NULL, \
}
@@ -299,8 +297,6 @@ static const u32 wb2_formats_rgb_yuv[] = {
.base = 0x1a00, .len = 0x100,}, \
.format_list = plane_formats_yuv, \
.num_formats = ARRAY_SIZE(plane_formats_yuv), \
- .virt_format_list = plane_formats, \
- .virt_num_formats = ARRAY_SIZE(plane_formats), \
.rotation_cfg = rot_cfg, \
}
@@ -310,8 +306,6 @@ static const u32 wb2_formats_rgb_yuv[] = {
.maxupscale = SSPP_UNITY_SCALE, \
.format_list = plane_formats, \
.num_formats = ARRAY_SIZE(plane_formats), \
- .virt_format_list = plane_formats, \
- .virt_num_formats = ARRAY_SIZE(plane_formats), \
}
#define _DMA_SBLK() \
@@ -320,8 +314,6 @@ static const u32 wb2_formats_rgb_yuv[] = {
.maxupscale = SSPP_UNITY_SCALE, \
.format_list = plane_formats, \
.num_formats = ARRAY_SIZE(plane_formats), \
- .virt_format_list = plane_formats, \
- .virt_num_formats = ARRAY_SIZE(plane_formats), \
}
static const struct dpu_rotation_cfg dpu_rot_sc7280_cfg_v2 = {
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 37e18e820a20..3f2646955ae0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -372,8 +372,6 @@ struct dpu_caps {
* @csc_blk:
* @format_list: Pointer to list of supported formats
* @num_formats: Number of supported formats
- * @virt_format_list: Pointer to list of supported formats for virtual planes
- * @virt_num_formats: Number of supported formats for virtual planes
* @dpu_rotation_cfg: inline rotation configuration
*/
struct dpu_sspp_sub_blks {
@@ -386,8 +384,6 @@ struct dpu_sspp_sub_blks {
const u32 *format_list;
u32 num_formats;
- const u32 *virt_format_list;
- u32 virt_num_formats;
const struct dpu_rotation_cfg *rotation_cfg;
};
--
2.39.2
[PATCH v5 07/12] drm/msm/dpu: move scaling limitations out of the hw_catalog
Max upscale / downscale factors are constant between platforms. In
preparation to adding support for virtual planes and allocating SSPP
blocks on demand move max scaling factors out of the HW catalog and
handle them in the dpu_plane directly. If any of the scaling blocks gets
different limitations, this will have to be handled separately, after
the plane refactoring.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 12
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 4
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 16 +---
3 files changed, 13 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index a3d29c69bda4..e9fd6b36a1c4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -113,10 +113,6 @@
#define MAX_HORZ_DECIMATION4
#define MAX_VERT_DECIMATION4
-#define MAX_UPSCALE_RATIO 20
-#define MAX_DOWNSCALE_RATIO4
-#define SSPP_UNITY_SCALE 1
-
#define STRCAT(X, Y) (X Y)
static const uint32_t plane_formats[] = {
@@ -274,8 +270,6 @@ static const u32 wb2_formats_rgb_yuv[] = {
/* SSPP common configuration */
#define _VIG_SBLK(scaler_ver) \
{ \
- .maxdwnscale = MAX_DOWNSCALE_RATIO, \
- .maxupscale = MAX_UPSCALE_RATIO, \
.scaler_blk = {.name = "scaler", \
.version = scaler_ver, \
.base = 0xa00, .len = 0xa0,}, \
@@ -288,8 +282,6 @@ static const u32 wb2_formats_rgb_yuv[] = {
#define _VIG_SBLK_ROT(scaler_ver, rot_cfg) \
{ \
- .maxdwnscale = MAX_DOWNSCALE_RATIO, \
- .maxupscale = MAX_UPSCALE_RATIO, \
.scaler_blk = {.name = "scaler", \
.version = scaler_ver, \
.base = 0xa00, .len = 0xa0,}, \
@@ -302,16 +294,12 @@ static const u32 wb2_formats_rgb_yuv[] = {
#define _VIG_SBLK_NOSCALE() \
{ \
- .maxdwnscale = SSPP_UNITY_SCALE, \
- .maxupscale = SSPP_UNITY_SCALE, \
.format_list = plane_formats, \
.num_formats = ARRAY_SIZE(plane_formats), \
}
#define _DMA_SBLK() \
{ \
- .maxdwnscale = SSPP_UNITY_SCALE, \
- .maxupscale = SSPP_UNITY_SCALE, \
.format_list = plane_formats, \
.num_formats = ARRAY_SIZE(plane_formats), \
}
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 3f2646955ae0..bf86d643887d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -364,8 +364,6 @@ struct dpu_caps {
/**
* struct dpu_sspp_sub_blks : SSPP sub-blocks
* common: Pointer to common configurations shared by sub blocks
- * @maxdwnscale: max downscale ratio supported(without DECIMATION)
- * @maxupscale: maxupscale ratio supported
* @max_per_pipe_bw: maximum allowable bandwidth of this pipe in kBps
* @qseed_ver: qseed version
* @scaler_blk:
@@ -375,8 +373,6 @@ struct dpu_caps {
* @dpu_rotation_cfg: inline rotation configuration
*/
struct dpu_sspp_sub_blks {
- u32 maxdwnscale;
- u32 maxupscale;
u32 max_per_pipe_bw;
u32 qseed_ver;
struct dpu_scaler_blk scaler_blk;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 325af392e6a2..115c1bd77bdd 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -785,12 +785,15 @@ static int dpu_plane_atomic_check_pipe(struct dpu_plane
*pdpu,
return 0;
}
+#define MAX_UPSCALE_RATIO 20
+#define MAX_DOWNSCALE_RATIO4
+
static int dpu_plane_atomic_check(struct drm_plane *plane,
struct drm_atomic_state *state)
{
struct drm_plane_state *new_plane_state =
drm_atomic_get_new_plane_state(state,
plane);
- int ret = 0, min_scale;
+ int ret = 0, min_scale, max_scale;
struct dpu_plane *pdpu = to_dpu_plane(plane);
struct dpu_kms *kms = _dpu_plane_get_kms(>base);
u64 max_mdp_clk_rate = kms->perf.max_core_clk_rate;
@@ -821,10 +824,17 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
pipe_hw_caps = pipe->sspp->cap;
sblk = pipe->sspp->cap->sblk;
- min_scale = FRAC_16_16(1, sblk->maxupscale);
+ if (sblk->scaler_blk.len) {
+ min_scale = FRAC_16_16(1, MAX_UPSCALE_RATIO);
+ max_scale = MAX_DOWNSCALE_RATIO << 16;
+ } else {
+ min_scale = DRM_PLANE_NO_SCALING;
+ max_scale = DRM_PLANE_NO_SCALING;
+ }
+
ret = drm_atomic_helper_check_plane_state(new_plane_state, crtc_state,
min_scale,
-
[PATCH v5 04/12] drm/msm/dpu: use drm_rect_fp_to_int()
Use the drm_rect_fp_to_int() helper instead of using the hand-written code. Reviewed-by: Abhinav Kumar Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 7 +-- 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index 8f2759d16247..4abaf2bb8a08 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -837,13 +837,8 @@ static int dpu_plane_atomic_check(struct drm_plane *plane, return -EINVAL; } - pipe_cfg->src_rect = new_plane_state->src; - /* state->src is 16.16, src_rect is not */ - pipe_cfg->src_rect.x1 >>= 16; - pipe_cfg->src_rect.x2 >>= 16; - pipe_cfg->src_rect.y1 >>= 16; - pipe_cfg->src_rect.y2 >>= 16; + drm_rect_fp_to_int(_cfg->src_rect, _plane_state->src); pipe_cfg->dst_rect = new_plane_state->dst; -- 2.39.2
[PATCH v5 05/12] drm/msm/dpu: move pstate->pipe initialization to dpu_plane_atomic_check
In preparation for virtualized planes support, move pstate->pipe
initialization from dpu_plane_reset() to dpu_plane_atomic_check(). In
case of virtual planes the plane's pipe will not be known up to the
point of atomic_check() callback.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 25 +++--
1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 4abaf2bb8a08..325af392e6a2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -805,13 +805,22 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
uint32_t max_linewidth;
unsigned int rotation;
uint32_t supported_rotations;
- const struct dpu_sspp_cfg *pipe_hw_caps = pstate->pipe.sspp->cap;
- const struct dpu_sspp_sub_blks *sblk = pstate->pipe.sspp->cap->sblk;
+ const struct dpu_sspp_cfg *pipe_hw_caps;
+ const struct dpu_sspp_sub_blks *sblk;
if (new_plane_state->crtc)
crtc_state = drm_atomic_get_new_crtc_state(state,
new_plane_state->crtc);
+ pipe->sspp = dpu_rm_get_sspp(>rm, pdpu->pipe);
+ r_pipe->sspp = NULL;
+
+ if (!pipe->sspp)
+ return -EINVAL;
+
+ pipe_hw_caps = pipe->sspp->cap;
+ sblk = pipe->sspp->cap->sblk;
+
min_scale = FRAC_16_16(1, sblk->maxupscale);
ret = drm_atomic_helper_check_plane_state(new_plane_state, crtc_state,
min_scale,
@@ -828,7 +837,6 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
- r_pipe->sspp = NULL;
pstate->stage = DPU_STAGE_0 + pstate->base.normalized_zpos;
if (pstate->stage >= pdpu->catalog->caps->max_mixer_blendstages) {
@@ -1292,7 +1300,6 @@ static void dpu_plane_reset(struct drm_plane *plane)
{
struct dpu_plane *pdpu;
struct dpu_plane_state *pstate;
- struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
if (!plane) {
DPU_ERROR("invalid plane\n");
@@ -1314,16 +1321,6 @@ static void dpu_plane_reset(struct drm_plane *plane)
return;
}
- /*
-* Set the SSPP here until we have proper virtualized DPU planes.
-* This is the place where the state is allocated, so fill it fully.
-*/
- pstate->pipe.sspp = dpu_rm_get_sspp(_kms->rm, pdpu->pipe);
- pstate->pipe.multirect_index = DPU_SSPP_RECT_SOLO;
- pstate->pipe.multirect_mode = DPU_SSPP_MULTIRECT_NONE;
-
- pstate->r_pipe.sspp = NULL;
-
__drm_atomic_helper_plane_reset(plane, >base);
}
--
2.39.2
[PATCH v5 02/12] drm/msm/dpu: relax YUV requirements
YUV formats require only CSC to be enabled. Even decimated formats
should not require scaler. Relax the requirement and don't check for the
scaler block while checking if YUV format can be enabled.
Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support")
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 1c3a2657450c..148bd79bdcef 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -743,10 +743,9 @@ static int dpu_plane_atomic_check_pipe(struct dpu_plane
*pdpu,
min_src_size = MSM_FORMAT_IS_YUV(fmt) ? 2 : 1;
if (MSM_FORMAT_IS_YUV(fmt) &&
- (!pipe->sspp->cap->sblk->scaler_blk.len ||
-!pipe->sspp->cap->sblk->csc_blk.len)) {
+ !pipe->sspp->cap->sblk->csc_blk.len) {
DPU_DEBUG_PLANE(pdpu,
- "plane doesn't have scaler/csc for yuv\n");
+ "plane doesn't have csc for yuv\n");
return -EINVAL;
}
--
2.39.2
[PATCH v5 01/12] drm/msm/dpu: limit QCM2290 to RGB formats only
The QCM2290 doesn't have CSC blocks, so it can not support YUV formats
even on ViG blocks. Fix the formats declared by _VIG_SBLK_NOSCALE().
Fixes: 5334087ee743 ("drm/msm: add support for QCM2290 MDSS")
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index fc178ec73907..648c8d0a4c36 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -308,8 +308,8 @@ static const u32 wb2_formats_rgb_yuv[] = {
{ \
.maxdwnscale = SSPP_UNITY_SCALE, \
.maxupscale = SSPP_UNITY_SCALE, \
- .format_list = plane_formats_yuv, \
- .num_formats = ARRAY_SIZE(plane_formats_yuv), \
+ .format_list = plane_formats, \
+ .num_formats = ARRAY_SIZE(plane_formats), \
.virt_format_list = plane_formats, \
.virt_num_formats = ARRAY_SIZE(plane_formats), \
}
--
2.39.2
[PATCH v5 00/12] drm/msm/dpu: support virtual wide planes
As promised in the basic wide planes support ([1]) here comes a series supporting 2*max_linewidth for all the planes. Note: Unlike v1 and v2 this series finally includes support for additional planes - having more planes than the number of SSPP blocks. Note: this iteration features handling of rotation and reflection of the wide plane. However rot90 is still not tested: it is enabled on sc7280 and it only supports UBWC (tiled) framebuffers, it was quite low on my priority list. [1] https://patchwork.freedesktop.org/series/99909/ To: Rob Clark To: Abhinav Kumar To: Dmitry Baryshkov To: Sean Paul To: Marijn Suijten To: David Airlie To: Daniel Vetter Cc: linux-arm-...@vger.kernel.org Cc: dri-de...@lists.freedesktop.org Cc: freedreno@lists.freedesktop.org Cc: linux-ker...@vger.kernel.org Changes in v5: - Dropped extra dpu_kms instance from dpu_plane_atomic_check() (Abhinav) - Use DRM_PLANE_NO_SCALING instead of (1 << 16) (Abhinav) - Dropped excess returns documentation for dpu_rm_reserve_sspp() (Sui Jingfeng, Abhinav) - best_weght -> best_weight (Abhinav) - Moved drm_rect_width() call back to the the patch "split dpu_plane_atomic_check()" (Abhinav) - Got rid of saved_fmt / saved dimensions (Abhinav) - Expanded the commit message to describe SSPP allocation per CRTC id (Abhinav) - Added comment on why the size change also causes resource reallocation (Abhinav) - Dropeed several last "feature" patches, leaving only SSPP reallocation and using 2 SSPPs per plane for now. The rest will be submitted separately. Changes since v3: - Dropped the drm_atomic_helper_check_plane_noscale (Ville) - Reworked the scaling factor according to global value and then check if SSPP has scaler_blk later on. - Split drm_rect_fp_to_int from the rotation-related fix (Abhinav) Changes since v2: - Dropped the encoder-related parts, leave all resource allocation as is (Abhinav) - Significantly reworked the SSPP allocation code - Added debugging code to dump RM state in dri/N/state Changes since v1: - Fixed build error due to me missing one of fixups, it was left uncommitted. - Implementated proper handling of wide plane rotation & reflection. --- Dmitry Baryshkov (12): drm/msm/dpu: limit QCM2290 to RGB formats only drm/msm/dpu: relax YUV requirements drm/msm/dpu: take plane rotation into account for wide planes drm/msm/dpu: use drm_rect_fp_to_int() drm/msm/dpu: move pstate->pipe initialization to dpu_plane_atomic_check drm/msm/dpu: drop virt_formats from SSPP subblock configuration drm/msm/dpu: move scaling limitations out of the hw_catalog drm/msm/dpu: split dpu_plane_atomic_check() drm/msm/dpu: move rot90 checking to dpu_plane_atomic_check_pipe() drm/msm/dpu: add support for virtual planes drm/msm/dpu: allow using two SSPP blocks for a single plane drm/msm/dpu: include SSPP allocation state into the dumped state drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 50 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 24 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 8 - drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h| 2 + drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 10 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h| 4 + drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 539 - drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 18 +- drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 84 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 27 ++ 10 files changed, 621 insertions(+), 145 deletions(-) --- base-commit: 0fc4bfab2cd45f9acb86c4f04b5191e114e901ed change-id: 20240626-dpu-virtual-wide-beefb746a900 Best regards, -- Dmitry Baryshkov
Re: [PATCH] drm/msm/adreno: fix a7xx gpu init
On Wed, Jun 26, 2024 at 09:53:16AM GMT, Neil Armstrong wrote:
> The gpulist has twice the a6xx gpulist, replace the second one
> with the a7xx gpulist.
>
> Solves:
> msm_dpu ae01000.display-controller: Unknown GPU revision: 7.3.0.1
> msm_dpu ae01000.display-controller: Unknown GPU revision: 67.5.10.1
> msm_dpu ae01000.display-controller: Unknown GPU revision: 67.5.20.1
>
> on SM8450, SM8550 & SM8560.
>
> Fixes: 8ed322f632a9 ("drm/msm/adreno: Split up giant device table")
> Signed-off-by: Neil Armstrong
> ---
> drivers/gpu/drm/msm/adreno/adreno_device.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Dmitry Baryshkov
--
With best wishes
Dmitry
Re: [PATCH] dt-bindings: display/msm/gmu: fix the schema being not applied
On Tue, Jun 25, 2024 at 04:51:27PM GMT, Rob Herring wrote:
> On Sun, Jun 23, 2024 at 02:59:30PM +0200, Krzysztof Kozlowski wrote:
> > dtschema v2024.4, v2024.5 and maybe earlier do not select device nodes for
>
> That should be just since db9c05a08709 ("validator: Rework selecting
> schemas for validation") AKA the 6x speed up in v2024.04.
>
> > given binding validation if the schema contains compatible list with
> > pattern and a const fallback. This leads to binding being a no-op - not
> > being applied at all. Issue should be fixed in the dtschema but for now
> > add a work-around do the binding can be used against DTS validation.
>
> The issue is we only look at the first compatible. I'm testing out a fix
> and will apply it tomorrow assuming no issues. With that, I don't think
> we should apply this patch.
I think we ended up picking up the next iteration of the patch, but we
can probably land a revert later.
>
> >
> > Signed-off-by: Krzysztof Kozlowski
> >
> > ---
> >
> > Cc: Akhil P Oommen
> > ---
> > .../devicetree/bindings/display/msm/gmu.yaml | 12
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/display/msm/gmu.yaml
> > b/Documentation/devicetree/bindings/display/msm/gmu.yaml
> > index b3837368a260..8d1b515f59ec 100644
> > --- a/Documentation/devicetree/bindings/display/msm/gmu.yaml
> > +++ b/Documentation/devicetree/bindings/display/msm/gmu.yaml
> > @@ -17,6 +17,18 @@ description: |
> >management and support to improve power efficiency and reduce the load on
> >the CPU.
> >
> > +# dtschema does not select nodes based on pattern+const, so add custom
> > select
> > +# as a work-around:
> > +select:
> > + properties:
> > +compatible:
> > + contains:
> > +enum:
> > + - qcom,adreno-gmu
> > + - qcom,adreno-gmu-wrapper
> > + required:
> > +- compatible
> > +
> > properties:
> >compatible:
> > oneOf:
> > --
> > 2.43.0
> >
--
With best wishes
Dmitry
[PATCH RFC v3] drm/msm/dpu: Configure DP INTF/PHY selector
From: Bjorn Andersson
Some platforms provides a mechanism for configuring the mapping between
(one or two) DisplayPort intfs and their PHYs.
In particular SC8180X requires this to be configured, since on this
platform there are fewer controllers than PHYs.
The change implements the logic for optionally configuring which PHY
each of the DP INTFs should be connected to and marks the SC8180X DPU to
program 2 entries.
For now the request is simply to program the mapping 1:1, any support
for alternative mappings is left until the use case arrise.
Note that e.g. msm-4.14 unconditionally maps INTF 0 to PHY 0 on all
platforms, so perhaps this is needed in order to get DisplayPort working
on some other platforms as well.
Signed-off-by: Bjorn Andersson
Co-developed-by: Bjorn Andersson
Signed-off-by: Dmitry Baryshkov
---
Changes in v3:
- Expanded the commit message and in-code comment based on feedback from
Abhinav
- Fixed field masks for the affected register (Abhinav)
- Link to v2:
https://lore.kernel.org/r/20240613-dp-phy-sel-v2-1-99af348c9...@linaro.org
Changes in v2:
- Removed entry from the catalog.
- Reworked the interface of dpu_hw_dp_phy_intf_sel(). Pass two entries
for the PHYs instead of three entries.
- It seems the register isn't present on sdm845, enabled the callback
only for DPU >= 5.x
- Added a comment regarding the data being platform-specific.
- Link to v1:
https://lore.kernel.org/r/20230612221047.1886709-1-quic_bjora...@quicinc.com
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c | 39 +++---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h | 18 --
drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h | 7 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 12 -
4 files changed, 70 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
index 05e48cf4ec1d..a11fdbefc8d2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
@@ -231,8 +231,38 @@ static void dpu_hw_intf_audio_select(struct dpu_hw_mdp
*mdp)
DPU_REG_WRITE(c, HDMI_DP_CORE_SELECT, 0x1);
}
+static void dpu_hw_dp_phy_intf_sel(struct dpu_hw_mdp *mdp,
+ enum dpu_dp_phy_sel phys[2])
+{
+ struct dpu_hw_blk_reg_map *c = >hw;
+ unsigned int intf;
+ u32 sel = 0;
+
+ sel |= FIELD_PREP(MDP_DP_PHY_INTF_SEL_INTF0, phys[0]);
+ sel |= FIELD_PREP(MDP_DP_PHY_INTF_SEL_INTF1, phys[1]);
+
+ for (intf = 0; intf < 2; intf++) {
+ switch (phys[intf]) {
+ case DPU_DP_PHY_0:
+ sel |= FIELD_PREP(MDP_DP_PHY_INTF_SEL_PHY0, intf + 1);
+ break;
+ case DPU_DP_PHY_1:
+ sel |= FIELD_PREP(MDP_DP_PHY_INTF_SEL_PHY1, intf + 1);
+ break;
+ case DPU_DP_PHY_2:
+ sel |= FIELD_PREP(MDP_DP_PHY_INTF_SEL_PHY2, intf + 1);
+ break;
+ default:
+ /* ignore */
+ break;
+ }
+ }
+
+ DPU_REG_WRITE(c, MDP_DP_PHY_INTF_SEL, sel);
+}
+
static void _setup_mdp_ops(struct dpu_hw_mdp_ops *ops,
- unsigned long cap)
+ unsigned long cap, const struct dpu_mdss_version *mdss_rev)
{
ops->setup_split_pipe = dpu_hw_setup_split_pipe;
ops->setup_clk_force_ctrl = dpu_hw_setup_clk_force_ctrl;
@@ -245,6 +275,9 @@ static void _setup_mdp_ops(struct dpu_hw_mdp_ops *ops,
ops->get_safe_status = dpu_hw_get_safe_status;
+ if (mdss_rev->core_major_ver >= 5)
+ ops->dp_phy_intf_sel = dpu_hw_dp_phy_intf_sel;
+
if (cap & BIT(DPU_MDP_AUDIO_SELECT))
ops->intf_audio_select = dpu_hw_intf_audio_select;
}
@@ -252,7 +285,7 @@ static void _setup_mdp_ops(struct dpu_hw_mdp_ops *ops,
struct dpu_hw_mdp *dpu_hw_mdptop_init(struct drm_device *dev,
const struct dpu_mdp_cfg *cfg,
void __iomem *addr,
- const struct dpu_mdss_cfg *m)
+ const struct dpu_mdss_version *mdss_rev)
{
struct dpu_hw_mdp *mdp;
@@ -270,7 +303,7 @@ struct dpu_hw_mdp *dpu_hw_mdptop_init(struct drm_device
*dev,
* Assign ops
*/
mdp->caps = cfg;
- _setup_mdp_ops(>ops, mdp->caps->features);
+ _setup_mdp_ops(>ops, mdp->caps->features, mdss_rev);
return mdp;
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
index 6f3dc98087df..3a17e63b851c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h
@@ -67,6 +67,13 @@ struct dpu_vsync_source_cfg {
u32 vsync_source;
};
+enum dpu_dp_
Re: [PATCH RFC v2] drm/msm/dpu: Configure DP INTF/PHY selector
On Tue, 25 Jun 2024 at 22:28, Abhinav Kumar wrote: > > > > On 6/25/2024 12:26 PM, Abhinav Kumar wrote: > > > > > > On 6/24/2024 6:39 PM, Abhinav Kumar wrote: > >> > >> > >> On 6/13/2024 4:17 AM, Dmitry Baryshkov wrote: > >>> From: Bjorn Andersson > >>> > >>> Some platforms provides a mechanism for configuring the mapping between > >>> (one or two) DisplayPort intfs and their PHYs. > >>> > >>> In particular SC8180X provides this functionality, without a default > >>> configuration, resulting in no connection between its two external > >>> DisplayPort controllers and any PHYs. > >>> > >> > >> I have to cross-check internally about what makes it mandatory to > >> program this only for sc8180xp. We were not programming this so far > >> for any chipset and this register is present all the way from sm8150 > >> till xe10100 and all the chipsets do not have a correct default value > >> which makes me think whether this is required to be programmed. > >> > >> Will update this thread once I do. > >> > > > > Ok, I checked more. The reason this is mandatory for sc8180xp is the > > number of controllers is greater than number of PHYs needing this to be > > programmed. On all other chipsets its a 1:1 mapping. > > > > Correction, number of controllers is < number of PHYs. Thanks, I'll c your explanation to the commit message if you don't mind. > > > I am fine with the change once the genmap comment is addressed. -- With best wishes Dmitry
Re: [PATCH v2 2/2] Revert "drm/msm/a6xx: Poll for GBIF unhalt status in hw_init"
On Tue, 25 Jun 2024 at 21:54, Konrad Dybcio wrote:
>
> Commit c9707bcbd0f3 ("drm/msm/adreno: De-spaghettify the use of memory
ID is not present in next
> barriers") made some fixups relating to write arrival, ensuring that
> the GPU's memory interface has *really really really* been told to come
> out of reset. That in turn rendered the hacky commit being reverted no
> longer necessary.
>
> Get rid of it.
>
> This reverts commit b77532803d11f2b03efab2ebfd8c0061cd7f8b30.
b77532803d11 ("drm/msm/a6xx: Poll for GBIF unhalt status in hw_init")
>
> Signed-off-by: Konrad Dybcio
> ---
> drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 4
> 1 file changed, 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 4083d0cad782..03e23eef5126 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -867,10 +867,6 @@ static int hw_init(struct msm_gpu *gpu)
> gpu_read(gpu, REG_A6XX_RBBM_GBIF_HALT);
> }
>
> - /* Some GPUs are stubborn and take their sweet time to unhalt GBIF! */
> - if (adreno_is_a7xx(adreno_gpu) && a6xx_has_gbif(adreno_gpu))
> - spin_until(!gpu_read(gpu, REG_A6XX_GBIF_HALT_ACK));
> -
> gpu_write(gpu, REG_A6XX_RBBM_SECVID_TSB_CNTL, 0);
>
> if (adreno_is_a619_holi(adreno_gpu))
>
> --
> 2.45.2
>
--
With best wishes
Dmitry
Re: [PATCH v2 0/4] dt-bindings: display/msm/gpu: few cleanups
On Sun, 23 Jun 2024 22:02:59 +0200, Krzysztof Kozlowski wrote: > Changes since v1: > 1. Add tags > 2. New patches #3 and #4 > 3. Drop previous patch "dt-bindings: display/msm/gpu: constrain >reg/reg-names per variant", because I need to investigate more. > > v1: dt-bindings: display/msm/gpu: constrain reg/reg-names per variant > > [...] Applied, thanks! [1/4] dt-bindings: display/msm/gpu: constrain clocks in top-level https://gitlab.freedesktop.org/lumag/msm/-/commit/d6c7c411be78 [2/4] dt-bindings: display/msm/gpu: define reg-names in top-level https://gitlab.freedesktop.org/lumag/msm/-/commit/c808ece19640 [3/4] dt-bindings: display/msm/gpu: simplify compatible regex https://gitlab.freedesktop.org/lumag/msm/-/commit/6d69f8d37c85 [4/4] dt-bindings: display/msm/gpu: fix the schema being not applied https://gitlab.freedesktop.org/lumag/msm/-/commit/399af57ccca2 Best regards, -- Dmitry Baryshkov
Re: [PATCH v2 1/2] drm/bridge-connector: reset the HDMI connector state
On Tue, Jun 25, 2024 at 05:49:54PM GMT, Maxime Ripard wrote: > On Tue, Jun 25, 2024 at 06:05:33PM GMT, Dmitry Baryshkov wrote: > > On Tue, 25 Jun 2024 at 18:02, Maxime Ripard wrote: > > > > > > Hi, > > > > > > On Sun, Jun 23, 2024 at 08:40:12AM GMT, Dmitry Baryshkov wrote: > > > > On HDMI connectors which use drm_bridge_connector and DRM_BRIDGE_OP_HDMI > > > > IGT chokes on the max_bpc property in several kms_properties tests due > > > > to the the drm_bridge_connector failing to reset HDMI-related > > > > properties. > > > > > > > > Call __drm_atomic_helper_connector_hdmi_reset() if there is a > > > > the drm_bridge_connector has bridge_hdmi. > > > > > > > > Note, the __drm_atomic_helper_connector_hdmi_reset() is moved to > > > > drm_atomic_state_helper.c because drm_bridge_connector.c can not depend > > > > on DRM_DISPLAY_HDMI_STATE_HELPER. At the same time it is impossible to > > > > call this function from HDMI bridges, there is is no function that > > > > corresponds to the drm_connector_funcs::reset(). > > > > > > Why can't it depend on DRM_DISPLAY_HDMI_STATE_HELPER? > > > > Is it okay to have DRM_KMS_HELPER to depend unconditionally or select > > DRM_DISPLAY_HDMI_STATE_HELPER? > > No, but it's not clear to me why drm_bridge_connector is part of > DRM_KMS_HELPER? It doesn't seem to be called from the core but only > drivers, just like DRM_PANEL_BRIDGE which has a separate config option But then DRM_PANEL_BRIDGE is also linked into drm_kms_helper.ko. I can't really say that I understand the definition of various helper modules, but my taste tells me that code from drm/*.c should not depend directly on the code from drm/display/*.c. -- With best wishes Dmitry
Re: [PATCH v2 1/2] drm/bridge-connector: reset the HDMI connector state
On Tue, 25 Jun 2024 at 18:02, Maxime Ripard wrote: > > Hi, > > On Sun, Jun 23, 2024 at 08:40:12AM GMT, Dmitry Baryshkov wrote: > > On HDMI connectors which use drm_bridge_connector and DRM_BRIDGE_OP_HDMI > > IGT chokes on the max_bpc property in several kms_properties tests due > > to the the drm_bridge_connector failing to reset HDMI-related > > properties. > > > > Call __drm_atomic_helper_connector_hdmi_reset() if there is a > > the drm_bridge_connector has bridge_hdmi. > > > > Note, the __drm_atomic_helper_connector_hdmi_reset() is moved to > > drm_atomic_state_helper.c because drm_bridge_connector.c can not depend > > on DRM_DISPLAY_HDMI_STATE_HELPER. At the same time it is impossible to > > call this function from HDMI bridges, there is is no function that > > corresponds to the drm_connector_funcs::reset(). > > Why can't it depend on DRM_DISPLAY_HDMI_STATE_HELPER? Is it okay to have DRM_KMS_HELPER to depend unconditionally or select DRM_DISPLAY_HDMI_STATE_HELPER? -- With best wishes Dmitry
Re: [PATCH v2 0/4] MSM8937 MDP/DSI PHY enablement
On Sun, 23 Jun 2024 22:30:35 +0200, Barnabás Czémán wrote: > This patch series adds support for the MDP and DSI PHY as found on the > MSM8937 platform. > > Applied, thanks! [1/4] dt-bindings: display/msm: qcom, mdp5: Add msm8937 compatible https://gitlab.freedesktop.org/lumag/msm/-/commit/c94dc5feb494 [2/4] drm/msm/mdp5: Add MDP5 configuration for MSM8937 https://gitlab.freedesktop.org/lumag/msm/-/commit/13099cb03f98 [3/4] dt-bindings: msm: dsi-phy-28nm: Document msm8937 compatible https://gitlab.freedesktop.org/lumag/msm/-/commit/60bdbaaf1220 [4/4] drm/msm/dsi: Add phy configuration for MSM8937 https://gitlab.freedesktop.org/lumag/msm/-/commit/2df0161959d1 Best regards, -- Dmitry Baryshkov
Re: [PATCH v3] drm/msm/dpu: remove CRTC frame event callback registration
On Tue, 25 Jun 2024 01:38:25 +0300, Dmitry Baryshkov wrote: > The frame event callback is always set to dpu_crtc_frame_event_cb() (or > to NULL) and the data is always either the CRTC itself or NULL > (correpondingly). Thus drop the event callback registration, call the > dpu_crtc_frame_event_cb() directly and gate on the dpu_enc->crtc > assigned using dpu_encoder_assign_crtc(). > > > [...] Applied, thanks! [1/1] drm/msm/dpu: remove CRTC frame event callback registration https://gitlab.freedesktop.org/lumag/msm/-/commit/5b90752f9619 Best regards, -- Dmitry Baryshkov
Re: [PATCH] drm/msm/mdp5: Remove MDP_CAP_SRC_SPLIT from msm8x53_config
On Mon, 24 Jun 2024 00:26:01 +0200, Barnabás Czémán wrote: > Remove MDP_CAP_SRC_SPLIT from msm8x53_config because > it is not referenced in downstream. > > Applied, thanks! [1/1] drm/msm/mdp5: Remove MDP_CAP_SRC_SPLIT from msm8x53_config https://gitlab.freedesktop.org/lumag/msm/-/commit/a3a6b350eb6c Best regards, -- Dmitry Baryshkov
Re: [PATCH v2 2/2] drm/connector: automatically set immutable flag for max_bpc property
On Tue, Jun 25, 2024 at 03:58:25PM GMT, Maxime Ripard wrote:
> On Tue, Jun 25, 2024 at 10:23:14AM GMT, Dmitry Baryshkov wrote:
> > On Tue, 25 Jun 2024 at 10:19, Maxime Ripard wrote:
> > >
> > > Hi,
> > >
> > > On Tue, Jun 25, 2024 at 09:21:27AM GMT, Dmitry Baryshkov wrote:
> > > > On Tue, 25 Jun 2024 at 01:56, Abhinav Kumar
> > > > wrote:
> > > > >
> > > > >
> > > > >
> > > > > On 6/24/2024 3:46 PM, Dmitry Baryshkov wrote:
> > > > > > On Tue, 25 Jun 2024 at 01:39, Abhinav Kumar
> > > > > > wrote:
> > > > > >>
> > > > > >> + IGT dev
> > > > > >>
> > > > > >> On 6/22/2024 10:40 PM, Dmitry Baryshkov wrote:
> > > > > >>> With the introduction of the HDMI Connector framework the driver
> > > > > >>> might
> > > > > >>> end up creating the max_bpc property with min = max = 8. IGT
> > > > > >>> insists
> > > > > >>> that such properties carry the 'immutable' flag. Automatically
> > > > > >>> set the
> > > > > >>> flag if the driver asks for the max_bpc property with min == max.
> > > > > >>>
> > > > > >>
> > > > > >> This change does not look right to me.
> > > > > >>
> > > > > >> I wonder why we need this check because DRM_MODE_PROP_IMMUTABLE
> > > > > >> means
> > > > > >> that as per the doc, userspace cannot change the property.
> > > > > >>
> > > > > >>* DRM_MODE_PROP_IMMUTABLE
> > > > > >>* Set for properties whose values cannot be changed
> > > > > >> by
> > > > > >>* userspace. The kernel is allowed to update the
> > > > > >> value of
> > > > > >> these
> > > > > >>* properties. This is generally used to expose
> > > > > >> probe state to
> > > > > >>* userspace, e.g. the EDID, or the connector path
> > > > > >> property
> > > > > >> on DP
> > > > > >>* MST sinks. Kernel can update the value of an
> > > > > >> immutable
> > > > > >> property
> > > > > >>* by calling drm_object_property_set_value().
> > > > > >>*/
> > > > > >>
> > > > > >> Here we are allowing userspace to change max_bpc
> > > > > >>
> > > > > >>
> > > > > >> drm_atomic_connector_set_property()
> > > > > >> {
> > > > > >> **
> > > > > >>
> > > > > >> } else if (property == connector->max_bpc_property) {
> > > > > >> state->max_requested_bpc = val;
> > > > > >>
> > > > > >> **
> > > > > >> }
> > > > > >>
> > > > > >> I believe you are referring to this IGT check right?
> > > > > >>
> > > > > >> https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tests/kms_properties.c#L428
> > > > > >
> > > > > > Yes
> > > > > >
> > > > > >>
> > > > > >> I think we should fix IGT in this case unless there is some reason
> > > > > >> we
> > > > > >> are missing. Because just because it has the same min and max does
> > > > > >> not
> > > > > >> mean its immutable by the doc of the IMMUTABLE flag.
> > > > > >
> > > > > > Well, having the same min and max means that it is impossible to
> > > > > > change the property. So the property is immutable, but doesn't have
> > > > > > the flag.
> > > > > >
> > > > >
> > > > > True, then does DRM_MODE_PROP_IMMUTABLE need a doc update too
> > > > > indicating
> > > > > that even if the min and max is same, property will be interpreted as
> > > > > immutable.
> > > >
> > > > Granted that I'm only doing it for max_bpc property I don't think so.
> > >
> > > Yeah, I have to agree with Abhinav here, it does look fishy to me too,
> > > even more so that it's only ever "documented" through an igt routine
> > > that has never documented why we want that.
> > >
> > > I'm fine with the change if it's indeed what we expect, and it might
> > > very well be, but I'd like to clear that up and document it first.
> >
> > Should I also move the setting of the IMMUTABLE flag to a more generic code?
>
> Possibly, but I guess that will depend on the outcome of that discussion
Agreed, I'll post it later today. Could you please ack or comment the first
patch?
--
With best wishes
Dmitry
Re: [PATCH v2 2/2] drm/connector: automatically set immutable flag for max_bpc property
On Tue, 25 Jun 2024 at 10:19, Maxime Ripard wrote:
>
> Hi,
>
> On Tue, Jun 25, 2024 at 09:21:27AM GMT, Dmitry Baryshkov wrote:
> > On Tue, 25 Jun 2024 at 01:56, Abhinav Kumar
> > wrote:
> > >
> > >
> > >
> > > On 6/24/2024 3:46 PM, Dmitry Baryshkov wrote:
> > > > On Tue, 25 Jun 2024 at 01:39, Abhinav Kumar
> > > > wrote:
> > > >>
> > > >> + IGT dev
> > > >>
> > > >> On 6/22/2024 10:40 PM, Dmitry Baryshkov wrote:
> > > >>> With the introduction of the HDMI Connector framework the driver might
> > > >>> end up creating the max_bpc property with min = max = 8. IGT insists
> > > >>> that such properties carry the 'immutable' flag. Automatically set the
> > > >>> flag if the driver asks for the max_bpc property with min == max.
> > > >>>
> > > >>
> > > >> This change does not look right to me.
> > > >>
> > > >> I wonder why we need this check because DRM_MODE_PROP_IMMUTABLE means
> > > >> that as per the doc, userspace cannot change the property.
> > > >>
> > > >>* DRM_MODE_PROP_IMMUTABLE
> > > >>* Set for properties whose values cannot be changed by
> > > >>* userspace. The kernel is allowed to update the value
> > > >> of
> > > >> these
> > > >>* properties. This is generally used to expose probe
> > > >> state to
> > > >>* userspace, e.g. the EDID, or the connector path
> > > >> property
> > > >> on DP
> > > >>* MST sinks. Kernel can update the value of an immutable
> > > >> property
> > > >>* by calling drm_object_property_set_value().
> > > >>*/
> > > >>
> > > >> Here we are allowing userspace to change max_bpc
> > > >>
> > > >>
> > > >> drm_atomic_connector_set_property()
> > > >> {
> > > >> **
> > > >>
> > > >> } else if (property == connector->max_bpc_property) {
> > > >> state->max_requested_bpc = val;
> > > >>
> > > >> **
> > > >> }
> > > >>
> > > >> I believe you are referring to this IGT check right?
> > > >>
> > > >> https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tests/kms_properties.c#L428
> > > >
> > > > Yes
> > > >
> > > >>
> > > >> I think we should fix IGT in this case unless there is some reason we
> > > >> are missing. Because just because it has the same min and max does not
> > > >> mean its immutable by the doc of the IMMUTABLE flag.
> > > >
> > > > Well, having the same min and max means that it is impossible to
> > > > change the property. So the property is immutable, but doesn't have
> > > > the flag.
> > > >
> > >
> > > True, then does DRM_MODE_PROP_IMMUTABLE need a doc update too indicating
> > > that even if the min and max is same, property will be interpreted as
> > > immutable.
> >
> > Granted that I'm only doing it for max_bpc property I don't think so.
>
> Yeah, I have to agree with Abhinav here, it does look fishy to me too,
> even more so that it's only ever "documented" through an igt routine
> that has never documented why we want that.
>
> I'm fine with the change if it's indeed what we expect, and it might
> very well be, but I'd like to clear that up and document it first.
Should I also move the setting of the IMMUTABLE flag to a more generic code?
--
With best wishes
Dmitry
Re: [PATCH v2 2/2] drm/connector: automatically set immutable flag for max_bpc property
On Tue, 25 Jun 2024 at 01:56, Abhinav Kumar wrote:
>
>
>
> On 6/24/2024 3:46 PM, Dmitry Baryshkov wrote:
> > On Tue, 25 Jun 2024 at 01:39, Abhinav Kumar
> > wrote:
> >>
> >> + IGT dev
> >>
> >> On 6/22/2024 10:40 PM, Dmitry Baryshkov wrote:
> >>> With the introduction of the HDMI Connector framework the driver might
> >>> end up creating the max_bpc property with min = max = 8. IGT insists
> >>> that such properties carry the 'immutable' flag. Automatically set the
> >>> flag if the driver asks for the max_bpc property with min == max.
> >>>
> >>
> >> This change does not look right to me.
> >>
> >> I wonder why we need this check because DRM_MODE_PROP_IMMUTABLE means
> >> that as per the doc, userspace cannot change the property.
> >>
> >>* DRM_MODE_PROP_IMMUTABLE
> >>* Set for properties whose values cannot be changed by
> >>* userspace. The kernel is allowed to update the value of
> >> these
> >>* properties. This is generally used to expose probe state
> >> to
> >>* userspace, e.g. the EDID, or the connector path property
> >> on DP
> >>* MST sinks. Kernel can update the value of an immutable
> >> property
> >>* by calling drm_object_property_set_value().
> >>*/
> >>
> >> Here we are allowing userspace to change max_bpc
> >>
> >>
> >> drm_atomic_connector_set_property()
> >> {
> >> **
> >>
> >> } else if (property == connector->max_bpc_property) {
> >> state->max_requested_bpc = val;
> >>
> >> **
> >> }
> >>
> >> I believe you are referring to this IGT check right?
> >>
> >> https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tests/kms_properties.c#L428
> >
> > Yes
> >
> >>
> >> I think we should fix IGT in this case unless there is some reason we
> >> are missing. Because just because it has the same min and max does not
> >> mean its immutable by the doc of the IMMUTABLE flag.
> >
> > Well, having the same min and max means that it is impossible to
> > change the property. So the property is immutable, but doesn't have
> > the flag.
> >
>
> True, then does DRM_MODE_PROP_IMMUTABLE need a doc update too indicating
> that even if the min and max is same, property will be interpreted as
> immutable.
Granted that I'm only doing it for max_bpc property I don't think so.
>
> >>
> >>
> >>> Fixes: aadb3e16b8f3 ("drm/connector: hdmi: Add output BPC to the
> >>> connector state")
> >>> Signed-off-by: Dmitry Baryshkov
> >>> ---
> >>>drivers/gpu/drm/drm_connector.c | 7 ++-
> >>>1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > With best wishes
> > Dmitry
--
With best wishes
Dmitry
Re: [PATCH v2 2/2] drm/connector: automatically set immutable flag for max_bpc property
On Tue, 25 Jun 2024 at 01:39, Abhinav Kumar wrote:
>
> + IGT dev
>
> On 6/22/2024 10:40 PM, Dmitry Baryshkov wrote:
> > With the introduction of the HDMI Connector framework the driver might
> > end up creating the max_bpc property with min = max = 8. IGT insists
> > that such properties carry the 'immutable' flag. Automatically set the
> > flag if the driver asks for the max_bpc property with min == max.
> >
>
> This change does not look right to me.
>
> I wonder why we need this check because DRM_MODE_PROP_IMMUTABLE means
> that as per the doc, userspace cannot change the property.
>
> * DRM_MODE_PROP_IMMUTABLE
> * Set for properties whose values cannot be changed by
> * userspace. The kernel is allowed to update the value of
> these
> * properties. This is generally used to expose probe state to
> * userspace, e.g. the EDID, or the connector path property
> on DP
> * MST sinks. Kernel can update the value of an immutable
> property
> * by calling drm_object_property_set_value().
> */
>
> Here we are allowing userspace to change max_bpc
>
>
> drm_atomic_connector_set_property()
> {
> **
>
> } else if (property == connector->max_bpc_property) {
> state->max_requested_bpc = val;
>
> **
> }
>
> I believe you are referring to this IGT check right?
>
> https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tests/kms_properties.c#L428
Yes
>
> I think we should fix IGT in this case unless there is some reason we
> are missing. Because just because it has the same min and max does not
> mean its immutable by the doc of the IMMUTABLE flag.
Well, having the same min and max means that it is impossible to
change the property. So the property is immutable, but doesn't have
the flag.
>
>
> > Fixes: aadb3e16b8f3 ("drm/connector: hdmi: Add output BPC to the connector
> > state")
> > Signed-off-by: Dmitry Baryshkov
> > ---
> > drivers/gpu/drm/drm_connector.c | 7 ++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
With best wishes
Dmitry
[PATCH v3] drm/msm/dpu: remove CRTC frame event callback registration
The frame event callback is always set to dpu_crtc_frame_event_cb() (or
to NULL) and the data is always either the CRTC itself or NULL
(correpondingly). Thus drop the event callback registration, call the
dpu_crtc_frame_event_cb() directly and gate on the dpu_enc->crtc
assigned using dpu_encoder_assign_crtc().
Signed-off-by: Dmitry Baryshkov
---
Changes in v3:
- Fixed documentation for dpu_crtc_frame_event_cb() to stop mentioning
registration. (Abhinav)
- Link to v2:
https://lore.kernel.org/dri-devel/20231005220659.2404199-1-dmitry.barysh...@linaro.org/
Changes in v2:
- Rebased on top of linux-next
- Link to v1:
https://lore.kernel.org/dri-devel/20230102154748.951328-1-dmitry.barysh...@linaro.org/
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 25 +++---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h| 2 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 41 +
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 10 ---
drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 4 ---
5 files changed, 18 insertions(+), 64 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 9f2164782844..4c1be2f0555f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -658,18 +658,18 @@ static void dpu_crtc_frame_event_work(struct kthread_work
*work)
DPU_ATRACE_END("crtc_frame_event");
}
-/*
- * dpu_crtc_frame_event_cb - crtc frame event callback API. CRTC module
- * registers this API to encoder for all frame event callbacks like
- * frame_error, frame_done, idle_timeout, etc. Encoder may call different
events
- * from different context - IRQ, user thread, commit_thread, etc. Each event
- * should be carefully reviewed and should be processed in proper task context
- * to avoid schedulin delay or properly manage the irq context's bottom half
- * processing.
+/**
+ * dpu_crtc_frame_event_cb - crtc frame event callback API
+ * @crtc: Pointer to crtc
+ * @event: Event to process
+ *
+ * Encoder may call this for different events from different context - IRQ,
+ * user thread, commit_thread, etc. Each event should be carefully reviewed and
+ * should be processed in proper task context to avoid schedulin delay or
+ * properly manage the irq context's bottom half processing.
*/
-static void dpu_crtc_frame_event_cb(void *data, u32 event)
+void dpu_crtc_frame_event_cb(struct drm_crtc *crtc, u32 event)
{
- struct drm_crtc *crtc = (struct drm_crtc *)data;
struct dpu_crtc *dpu_crtc;
struct msm_drm_private *priv;
struct dpu_crtc_frame_event *fevent;
@@ -1091,9 +1091,6 @@ static void dpu_crtc_disable(struct drm_crtc *crtc,
dpu_core_perf_crtc_update(crtc, 0);
- drm_for_each_encoder_mask(encoder, crtc->dev, crtc->state->encoder_mask)
- dpu_encoder_register_frame_event_callback(encoder, NULL, NULL);
-
memset(cstate->mixers, 0, sizeof(cstate->mixers));
cstate->num_mixers = 0;
@@ -1132,8 +1129,6 @@ static void dpu_crtc_enable(struct drm_crtc *crtc,
*/
if (dpu_encoder_get_intf_mode(encoder) == INTF_MODE_VIDEO)
request_bandwidth = true;
- dpu_encoder_register_frame_event_callback(encoder,
- dpu_crtc_frame_event_cb, (void *)crtc);
}
if (request_bandwidth)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index 539b68b1626a..b26d5fe40c72 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -300,4 +300,6 @@ static inline enum dpu_crtc_client_type
dpu_crtc_get_client_type(
return crtc && crtc->state ? RT_CLIENT : NRT_CLIENT;
}
+void dpu_crtc_frame_event_cb(struct drm_crtc *crtc, u32 event);
+
#endif /* _DPU_CRTC_H_ */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 708657598cce..4099e72820f9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -151,8 +151,6 @@ enum dpu_enc_rc_states {
* @frame_busy_mask: Bitmask tracking which phys_enc we are still
* busy processing current command.
* Bit0 = phys_encs[0] etc.
- * @crtc_frame_event_cb: callback handler for frame event
- * @crtc_frame_event_cb_data: callback handler private data
* @frame_done_timeout_ms: frame done timeout in ms
* @frame_done_timeout_cnt:atomic counter tracking the number of frame
* done timeouts
@@ -192,8 +190,6 @@ struct dpu_encoder_virt {
struct mutex enc_lock;
DECLARE_BITMAP(frame_busy_mask, MAX_PHYS_ENCODERS_PER_VIRTUAL);
- void (*crtc_frame_event_cb)(void *, u32 event);
- void *cr
Re: [PATCH v2 1/2] drm/bridge-connector: reset the HDMI connector state
On Tue, 25 Jun 2024 at 01:28, Abhinav Kumar wrote:
>
>
>
> On 6/22/2024 10:40 PM, Dmitry Baryshkov wrote:
> > On HDMI connectors which use drm_bridge_connector and DRM_BRIDGE_OP_HDMI
> > IGT chokes on the max_bpc property in several kms_properties tests due
> > to the the drm_bridge_connector failing to reset HDMI-related
> > properties.
> >
> > Call __drm_atomic_helper_connector_hdmi_reset() if there is a
> > the drm_bridge_connector has bridge_hdmi.
> >
> > Note, the __drm_atomic_helper_connector_hdmi_reset() is moved to
> > drm_atomic_state_helper.c because drm_bridge_connector.c can not depend
> > on DRM_DISPLAY_HDMI_STATE_HELPER. At the same time it is impossible to
> > call this function from HDMI bridges, there is is no function that
> > corresponds to the drm_connector_funcs::reset().
> >
> > Fixes: 6b4468b0c6ba ("drm/bridge-connector: implement glue code for HDMI
> > connector")
> > Signed-off-by: Dmitry Baryshkov
> > ---
> > drivers/gpu/drm/display/drm_hdmi_state_helper.c | 21 -
> > drivers/gpu/drm/drm_atomic_state_helper.c | 21 +
> > drivers/gpu/drm/drm_bridge_connector.c | 13 -
> > include/drm/display/drm_hdmi_state_helper.h | 3 ---
> > include/drm/drm_atomic_state_helper.h | 2 ++
> > 5 files changed, 35 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> > b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> > index 2dab3ad8ce64..67f39857b0b4 100644
> > --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> > +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> > @@ -8,27 +8,6 @@
> > #include
> > #include
> >
> > -/**
> > - * __drm_atomic_helper_connector_hdmi_reset() - Initializes all HDMI
> > @drm_connector_state resources
> > - * @connector: DRM connector
> > - * @new_conn_state: connector state to reset
> > - *
> > - * Initializes all HDMI resources from a @drm_connector_state without
> > - * actually allocating it. This is useful for HDMI drivers, in
> > - * combination with __drm_atomic_helper_connector_reset() or
> > - * drm_atomic_helper_connector_reset().
> > - */
> > -void __drm_atomic_helper_connector_hdmi_reset(struct drm_connector
> > *connector,
> > - struct drm_connector_state
> > *new_conn_state)
> > -{
> > - unsigned int max_bpc = connector->max_bpc;
> > -
> > - new_conn_state->max_bpc = max_bpc;
> > - new_conn_state->max_requested_bpc = max_bpc;
> > - new_conn_state->hdmi.broadcast_rgb = DRM_HDMI_BROADCAST_RGB_AUTO;
> > -}
> > -EXPORT_SYMBOL(__drm_atomic_helper_connector_hdmi_reset);
> > -
> > static const struct drm_display_mode *
> > connector_state_get_mode(const struct drm_connector_state *conn_state)
> > {
> > diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c
> > b/drivers/gpu/drm/drm_atomic_state_helper.c
> > index 519228eb1095..1518ada81b45 100644
> > --- a/drivers/gpu/drm/drm_atomic_state_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_state_helper.c
> > @@ -478,6 +478,27 @@ void drm_atomic_helper_connector_reset(struct
> > drm_connector *connector)
> > }
> > EXPORT_SYMBOL(drm_atomic_helper_connector_reset);
> >
> > +/**
> > + * __drm_atomic_helper_connector_hdmi_reset() - Initializes all HDMI
> > @drm_connector_state resources
> > + * @connector: DRM connector
> > + * @new_conn_state: connector state to reset
> > + *
> > + * Initializes all HDMI resources from a @drm_connector_state without
> > + * actually allocating it. This is useful for HDMI drivers, in
> > + * combination with __drm_atomic_helper_connector_reset() or
> > + * drm_atomic_helper_connector_reset().
> > + */
> > +void __drm_atomic_helper_connector_hdmi_reset(struct drm_connector
> > *connector,
> > + struct drm_connector_state
> > *new_conn_state)
> > +{
> > + unsigned int max_bpc = connector->max_bpc;
> > +
> > + new_conn_state->max_bpc = max_bpc;
> > + new_conn_state->max_requested_bpc = max_bpc;
>
> I understand this is just code propagation but do we need a max_bpc
> local variable?
>
> We can just do
>
> new_conn_state->max_bpc = connector->max_bpc;
> new_conn_state->max_requested_bpc = connector->max_bpc;
Possible implementation, but I was really just doing code move.
>
> But apart from that nit, this LGTM.
>
> Reviewed-by: Abhinav Kumar
--
With best wishes
Dmitry
[PATCH v5 10/16] drm/msm/dpu: move pitch check to _dpu_format_get_plane_sizes_linear()
The _dpu_format_get_plane_sizes_linear() already compares pitches of
the framebuffer with the calculated pitches. Move the check to the same
place, demoting DPU_ERROR to DPU_DEBUG to prevent user from spamming the
kernel log.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c | 16 ++--
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
index df046bc88715..4d17eb88af40 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
@@ -229,8 +229,13 @@ static int _dpu_format_get_plane_sizes_linear(
* all the components based on ubwc specifications.
*/
for (i = 0; i < layout->num_planes && i < DPU_MAX_PLANES; ++i) {
- if (layout->plane_pitch[i] < fb->pitches[i])
+ if (layout->plane_pitch[i] <= fb->pitches[i]) {
layout->plane_pitch[i] = fb->pitches[i];
+ } else {
+ DRM_DEBUG("plane %u expected pitch %u, fb %u\n",
+ i, layout->plane_pitch[i], fb->pitches[i]);
+ return -EINVAL;
+ }
}
for (i = 0; i < DPU_MAX_PLANES; i++)
@@ -360,15 +365,6 @@ static int _dpu_format_populate_addrs_linear(
{
unsigned int i;
- /* Can now check the pitches given vs pitches expected */
- for (i = 0; i < layout->num_planes; ++i) {
- if (layout->plane_pitch[i] > fb->pitches[i]) {
- DRM_ERROR("plane %u expected pitch %u, fb %u\n",
- i, layout->plane_pitch[i], fb->pitches[i]);
- return -EINVAL;
- }
- }
-
/* Populate addresses for simple formats here */
for (i = 0; i < layout->num_planes; ++i) {
layout->plane_addr[i] = msm_framebuffer_iova(fb, aspace, i);
--
2.39.2
[PATCH v5 13/16] drm/msm/dpu: move layout setup population out of dpu_plane_prepare_fb()
Move the call to dpu_format_populate_plane_sizes() to the atomic_check
step, so that any issues with the FB layout can be reported as early as
possible.
At the same time move the call to dpu_format_populate_addrs() to
dpu_plane_sspp_atomic_update(). This way the all layout management is
performed only for the visible planes: the .prepare_fb callback is
called for not visible planes too, so keeping dpu_format_populate_addrs
in dpu_plane_prepare_fb() will require dpu_format_populate_plane_sizes()
to be called for !visible planes too.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 26 +++---
1 file changed, 11 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 1431ea753a4f..a309b06b0992 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -674,19 +674,6 @@ static int dpu_plane_prepare_fb(struct drm_plane *plane,
}
}
- ret = dpu_format_populate_plane_sizes(new_state->fb, >layout);
- if (ret) {
- DPU_ERROR_PLANE(pdpu, "failed to get format plane sizes, %d\n",
ret);
- if (pstate->aspace)
- msm_framebuffer_cleanup(new_state->fb, pstate->aspace,
- pstate->needs_dirtyfb);
- return ret;
- }
-
- dpu_format_populate_addrs(pstate->aspace,
- new_state->fb,
- >layout);
-
return 0;
}
@@ -863,6 +850,12 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
return -E2BIG;
}
+ ret = dpu_format_populate_plane_sizes(new_plane_state->fb,
>layout);
+ if (ret) {
+ DPU_ERROR_PLANE(pdpu, "failed to get format plane sizes, %d\n",
ret);
+ return ret;
+ }
+
fmt = msm_framebuffer_format(new_plane_state->fb);
max_linewidth = pdpu->catalog->caps->max_linewidth;
@@ -1090,7 +1083,8 @@ static void dpu_plane_sspp_update_pipe(struct drm_plane
*plane,
_dpu_plane_set_qos_remap(plane, pipe);
}
-static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)
+static void dpu_plane_sspp_atomic_update(struct drm_plane *plane,
+struct drm_plane_state *new_state)
{
struct dpu_plane *pdpu = to_dpu_plane(plane);
struct drm_plane_state *state = plane->state;
@@ -,6 +1105,8 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane
*plane)
pstate->needs_qos_remap |= (is_rt_pipe != pdpu->is_rt_pipe);
pdpu->is_rt_pipe = is_rt_pipe;
+ dpu_format_populate_addrs(pstate->aspace, new_state->fb,
>layout);
+
DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u " DRM_RECT_FMT
", %p4cc ubwc %d\n", fb->base.id,
DRM_RECT_FP_ARG(>src),
crtc->base.id, DRM_RECT_ARG(>dst),
@@ -1175,7 +1171,7 @@ static void dpu_plane_atomic_update(struct drm_plane
*plane,
if (!new_state->visible) {
_dpu_plane_atomic_disable(plane);
} else {
- dpu_plane_sspp_atomic_update(plane);
+ dpu_plane_sspp_atomic_update(plane, new_state);
}
}
--
2.39.2
[PATCH v5 09/16] drm/msm/dpu: pass drm_framebuffer to _dpu_format_get_plane_sizes()
Instead of passing width / height / pitches, pass drm_framebuffer
directly. This allows us to drop the useless check for !pitches, since
an array can not be NULL.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c | 73 ++---
1 file changed, 34 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
index 46237a1ca6a5..df046bc88715 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
@@ -95,8 +95,7 @@ static int _dpu_format_get_media_color_ubwc(const struct
msm_format *fmt)
static int _dpu_format_get_plane_sizes_ubwc(
const struct msm_format *fmt,
- const uint32_t width,
- const uint32_t height,
+ struct drm_framebuffer *fb,
struct dpu_hw_fmt_layout *layout)
{
int i;
@@ -104,8 +103,8 @@ static int _dpu_format_get_plane_sizes_ubwc(
bool meta = MSM_FORMAT_IS_UBWC(fmt);
memset(layout, 0, sizeof(struct dpu_hw_fmt_layout));
- layout->width = width;
- layout->height = height;
+ layout->width = fb->width;
+ layout->height = fb->height;
layout->num_planes = fmt->num_planes;
color = _dpu_format_get_media_color_ubwc(fmt);
@@ -121,13 +120,13 @@ static int _dpu_format_get_plane_sizes_ubwc(
uint32_t uv_meta_scanlines = 0;
layout->num_planes = 2;
- layout->plane_pitch[0] = VENUS_Y_STRIDE(color, width);
- y_sclines = VENUS_Y_SCANLINES(color, height);
+ layout->plane_pitch[0] = VENUS_Y_STRIDE(color, fb->width);
+ y_sclines = VENUS_Y_SCANLINES(color, fb->height);
layout->plane_size[0] = MSM_MEDIA_ALIGN(layout->plane_pitch[0] *
y_sclines, DPU_UBWC_PLANE_SIZE_ALIGNMENT);
- layout->plane_pitch[1] = VENUS_UV_STRIDE(color, width);
- uv_sclines = VENUS_UV_SCANLINES(color, height);
+ layout->plane_pitch[1] = VENUS_UV_STRIDE(color, fb->width);
+ uv_sclines = VENUS_UV_SCANLINES(color, fb->height);
layout->plane_size[1] = MSM_MEDIA_ALIGN(layout->plane_pitch[1] *
uv_sclines, DPU_UBWC_PLANE_SIZE_ALIGNMENT);
@@ -135,13 +134,13 @@ static int _dpu_format_get_plane_sizes_ubwc(
goto done;
layout->num_planes += 2;
- layout->plane_pitch[2] = VENUS_Y_META_STRIDE(color, width);
- y_meta_scanlines = VENUS_Y_META_SCANLINES(color, height);
+ layout->plane_pitch[2] = VENUS_Y_META_STRIDE(color, fb->width);
+ y_meta_scanlines = VENUS_Y_META_SCANLINES(color, fb->height);
layout->plane_size[2] = MSM_MEDIA_ALIGN(layout->plane_pitch[2] *
y_meta_scanlines, DPU_UBWC_PLANE_SIZE_ALIGNMENT);
- layout->plane_pitch[3] = VENUS_UV_META_STRIDE(color, width);
- uv_meta_scanlines = VENUS_UV_META_SCANLINES(color, height);
+ layout->plane_pitch[3] = VENUS_UV_META_STRIDE(color, fb->width);
+ uv_meta_scanlines = VENUS_UV_META_SCANLINES(color, fb->height);
layout->plane_size[3] = MSM_MEDIA_ALIGN(layout->plane_pitch[3] *
uv_meta_scanlines, DPU_UBWC_PLANE_SIZE_ALIGNMENT);
@@ -150,16 +149,16 @@ static int _dpu_format_get_plane_sizes_ubwc(
layout->num_planes = 1;
- layout->plane_pitch[0] = VENUS_RGB_STRIDE(color, width);
- rgb_scanlines = VENUS_RGB_SCANLINES(color, height);
+ layout->plane_pitch[0] = VENUS_RGB_STRIDE(color, fb->width);
+ rgb_scanlines = VENUS_RGB_SCANLINES(color, fb->height);
layout->plane_size[0] = MSM_MEDIA_ALIGN(layout->plane_pitch[0] *
rgb_scanlines, DPU_UBWC_PLANE_SIZE_ALIGNMENT);
if (!meta)
goto done;
layout->num_planes += 2;
- layout->plane_pitch[2] = VENUS_RGB_META_STRIDE(color, width);
- rgb_meta_scanlines = VENUS_RGB_META_SCANLINES(color, height);
+ layout->plane_pitch[2] = VENUS_RGB_META_STRIDE(color,
fb->width);
+ rgb_meta_scanlines = VENUS_RGB_META_SCANLINES(color,
fb->height);
layout->plane_size[2] = MSM_MEDIA_ALIGN(layout->plane_pitch[2] *
rgb_meta_scanlines, DPU_UBWC_PLANE_SIZE_ALIGNMENT);
}
@@ -173,23 +172,21 @@ static int _dpu_format_get_plane_sizes_ubwc(
static int _dpu_format_get_plane_sizes_linear(
const struct msm_format *fmt,
- const uint32_t width,
- const uint32
[PATCH v5 14/16] drm/msm/dpu: check for the plane pitch overflow
Check that the plane pitch doesn't overflow the maximum pitch size
allowed by the hardware.
Reviewed-by: Abhinav Kumar
Tested-by: Abhinav Kumar # sc7280
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 2 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 6 +-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
index 4a910b808687..8998d1862e16 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
@@ -12,6 +12,8 @@
struct dpu_hw_sspp;
+#define DPU_SSPP_MAX_PITCH_SIZE0x
+
/**
* Flags
*/
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index a309b06b0992..a629eb3a6436 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -782,7 +782,7 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
{
struct drm_plane_state *new_plane_state =
drm_atomic_get_new_plane_state(state,
plane);
- int ret = 0, min_scale;
+ int i, ret = 0, min_scale;
struct dpu_plane *pdpu = to_dpu_plane(plane);
struct dpu_kms *kms = _dpu_plane_get_kms(>base);
u64 max_mdp_clk_rate = kms->perf.max_core_clk_rate;
@@ -856,6 +856,10 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
return ret;
}
+ for (i = 0; i < pstate->layout.num_planes; i++)
+ if (pstate->layout.plane_pitch[i] > DPU_SSPP_MAX_PITCH_SIZE)
+ return -E2BIG;
+
fmt = msm_framebuffer_format(new_plane_state->fb);
max_linewidth = pdpu->catalog->caps->max_linewidth;
--
2.39.2
[PATCH v5 08/16] drm/msm/dpu: drop msm_format from struct dpu_hw_fmt_layout
The struct dpu_hw_fmt_layout defines hardware data layout (addresses,
sizes and pitches. Drop format field from this structure as it's not a
part of the data layout.
Signed-off-by: Dmitry Baryshkov
---
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c| 31 +++---
drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c| 23
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h| 2 --
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c | 4 +--
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h | 3 ++-
5 files changed, 25 insertions(+), 38 deletions(-)
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 882c717859ce..c4a16a73bc97 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
@@ -166,10 +166,10 @@ static void dpu_encoder_phys_wb_set_qos(struct
dpu_encoder_phys *phys_enc)
/**
* dpu_encoder_phys_wb_setup_fb - setup output framebuffer
* @phys_enc: Pointer to physical encoder
- * @fb:Pointer to output framebuffer
+ * @format: Format of the framebuffer
*/
static void dpu_encoder_phys_wb_setup_fb(struct dpu_encoder_phys *phys_enc,
- struct drm_framebuffer *fb)
+const struct msm_format *format)
{
struct dpu_encoder_phys_wb *wb_enc = to_dpu_encoder_phys_wb(phys_enc);
struct dpu_hw_wb *hw_wb;
@@ -193,12 +193,12 @@ static void dpu_encoder_phys_wb_setup_fb(struct
dpu_encoder_phys *phys_enc,
hw_wb->ops.setup_roi(hw_wb, wb_cfg);
if (hw_wb->ops.setup_outformat)
- hw_wb->ops.setup_outformat(hw_wb, wb_cfg);
+ hw_wb->ops.setup_outformat(hw_wb, wb_cfg, format);
if (hw_wb->ops.setup_cdp) {
const struct dpu_perf_cfg *perf =
phys_enc->dpu_kms->catalog->perf;
- hw_wb->ops.setup_cdp(hw_wb, wb_cfg->dest.format,
+ hw_wb->ops.setup_cdp(hw_wb, format,
perf->cdp_cfg[DPU_PERF_CDP_USAGE_NRT].wr_enable);
}
@@ -318,15 +318,10 @@ static void dpu_encoder_phys_wb_setup(
{
struct dpu_hw_wb *hw_wb = phys_enc->hw_wb;
struct drm_display_mode mode = phys_enc->cached_mode;
- struct drm_framebuffer *fb = NULL;
struct dpu_encoder_phys_wb *wb_enc = to_dpu_encoder_phys_wb(phys_enc);
- struct drm_writeback_job *wb_job;
const struct msm_format *format;
- const struct msm_format *dpu_fmt;
- wb_job = wb_enc->wb_job;
format = msm_framebuffer_format(wb_enc->wb_job->fb);
- dpu_fmt = mdp_get_format(_enc->dpu_kms->base,
format->pixel_format, wb_job->fb->modifier);
DPU_DEBUG("[mode_set:%d, \"%s\",%d,%d]\n",
hw_wb->idx - WB_0, mode.name,
@@ -338,9 +333,9 @@ static void dpu_encoder_phys_wb_setup(
dpu_encoder_phys_wb_set_qos(phys_enc);
- dpu_encoder_phys_wb_setup_fb(phys_enc, fb);
+ dpu_encoder_phys_wb_setup_fb(phys_enc, format);
- dpu_encoder_helper_phys_setup_cdm(phys_enc, dpu_fmt,
CDM_CDWN_OUTPUT_WB);
+ dpu_encoder_helper_phys_setup_cdm(phys_enc, format, CDM_CDWN_OUTPUT_WB);
dpu_encoder_phys_wb_setup_ctl(phys_enc);
}
@@ -584,14 +579,6 @@ static void dpu_encoder_phys_wb_prepare_wb_job(struct
dpu_encoder_phys *phys_enc
format = msm_framebuffer_format(job->fb);
- wb_cfg->dest.format = mdp_get_format(_enc->dpu_kms->base,
-format->pixel_format,
job->fb->modifier);
- if (!wb_cfg->dest.format) {
- /* this error should be detected during atomic_check */
- DPU_ERROR("failed to get format %p4cc\n",
>pixel_format);
- return;
- }
-
ret = dpu_format_populate_layout(aspace, job->fb, _cfg->dest);
if (ret) {
DPU_DEBUG("failed to populate layout %d\n", ret);
@@ -600,10 +587,10 @@ static void dpu_encoder_phys_wb_prepare_wb_job(struct
dpu_encoder_phys *phys_enc
wb_cfg->dest.width = job->fb->width;
wb_cfg->dest.height = job->fb->height;
- wb_cfg->dest.num_planes = wb_cfg->dest.format->num_planes;
+ wb_cfg->dest.num_planes = format->num_planes;
- if ((wb_cfg->dest.format->fetch_type == MDP_PLANE_PLANAR) &&
- (wb_cfg->dest.format->element[0] == C1_B_Cb))
+ if ((format->fetch_type == MDP_PLANE_PLANAR) &&
+ (format->element[0] == C1_B_Cb))
swap(wb_cfg->dest.plane_addr[1], wb_cfg->dest.plane_addr[2]);
DPU_DEBUG("[fb_offset:%8.8x,%8.8x,%8.8x,%8.8x]\n",
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_fo
[PATCH v5 15/16] drm/msm/dpu: merge MAX_IMG_WIDTH/HEIGHT with DPU_MAX_IMG_WIDTH/HEIGHT
dpu_formats.c defines DPU_MAX_IMG_WIDTH and _HEIGHT, while
dpu_hw_catalog.h defines just MAX_IMG_WIDTH and _HEIGHT. Merge these
constants to remove duplication.
Reviewed-by: Abhinav Kumar
Tested-by: Abhinav Kumar # sc7280
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c| 3 ---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 4 ++--
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 4 ++--
3 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
index 095bb947f1ff..b0909cbd91cb 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
@@ -13,9 +13,6 @@
#define DPU_UBWC_PLANE_SIZE_ALIGNMENT 4096
-#define DPU_MAX_IMG_WIDTH 0x3FFF
-#define DPU_MAX_IMG_HEIGHT 0x3FFF
-
/*
* struct dpu_media_color_map - maps drm format to media format
* @format: DRM base pixel format
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 37e18e820a20..34e60483fbcf 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -21,8 +21,8 @@
#define DPU_HW_BLK_NAME_LEN16
-#define MAX_IMG_WIDTH 0x3fff
-#define MAX_IMG_HEIGHT 0x3fff
+#define DPU_MAX_IMG_WIDTH 0x3fff
+#define DPU_MAX_IMG_HEIGHT 0x3fff
#define CRTC_DUAL_MIXERS 2
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index a629eb3a6436..4712aa6f7929 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -843,8 +843,8 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
fb_rect.y2 = new_plane_state->fb->height;
/* Ensure fb size is supported */
- if (drm_rect_width(_rect) > MAX_IMG_WIDTH ||
- drm_rect_height(_rect) > MAX_IMG_HEIGHT) {
+ if (drm_rect_width(_rect) > DPU_MAX_IMG_WIDTH ||
+ drm_rect_height(_rect) > DPU_MAX_IMG_HEIGHT) {
DPU_DEBUG_PLANE(pdpu, "invalid framebuffer " DRM_RECT_FMT "\n",
DRM_RECT_ARG(_rect));
return -E2BIG;
--
2.39.2
[PATCH v5 07/16] drm/msm/dpu: drop extra aspace checks in dpu_formats
The DPU driver isn't expected to be used without an IOMMU. Thus the
aspace will be always present. Not to mention that mdp4/mdp5 drivers
call msm_framebuffer_iova() without such checks, as the whole
msm_framebuffer layer is expected to support both IOMMU and IOMMU-less
configurations.
Drop these useless if (aspace) checks.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
index 027eb5ecff08..8c2dc5b59bb0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
@@ -280,8 +280,7 @@ static int _dpu_format_populate_addrs_ubwc(
return -EINVAL;
}
- if (aspace)
- base_addr = msm_framebuffer_iova(fb, aspace, 0);
+ base_addr = msm_framebuffer_iova(fb, aspace, 0);
if (!base_addr) {
DRM_ERROR("failed to retrieve base addr\n");
return -EFAULT;
@@ -376,9 +375,7 @@ static int _dpu_format_populate_addrs_linear(
/* Populate addresses for simple formats here */
for (i = 0; i < layout->num_planes; ++i) {
- if (aspace)
- layout->plane_addr[i] =
- msm_framebuffer_iova(fb, aspace, i);
+ layout->plane_addr[i] = msm_framebuffer_iova(fb, aspace, i);
if (!layout->plane_addr[i]) {
DRM_ERROR("failed to retrieve base addr\n");
return -EFAULT;
--
2.39.2
[PATCH v5 16/16] drm/msm/dpu: sync mode_config limits to the FB limits in dpu_plane.c
Lift mode_config limits set by the DPU driver to the actual FB limits as
handled by the dpu_plane.c. Move 2*max_lm_width check where it belongs,
to the drm_crtc_helper_funcs::mode_valid() callback.
Reviewed-by: Abhinav Kumar
Tested-by: Abhinav Kumar # sc7280
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 15 +++
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 9 ++---
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 2a87dd7188b8..f4ec3df45536 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1235,6 +1235,20 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
return 0;
}
+static enum drm_mode_status dpu_crtc_mode_valid(struct drm_crtc *crtc,
+ const struct drm_display_mode
*mode)
+{
+ struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
+
+ /*
+* max crtc width is equal to the max mixer width * 2 and max height is
+* is 4K
+*/
+ return drm_mode_validate_size(mode,
+ 2 *
dpu_kms->catalog->caps->max_mixer_width,
+ 4096);
+}
+
int dpu_crtc_vblank(struct drm_crtc *crtc, bool en)
{
struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
@@ -1450,6 +1464,7 @@ static const struct drm_crtc_helper_funcs
dpu_crtc_helper_funcs = {
.atomic_check = dpu_crtc_atomic_check,
.atomic_begin = dpu_crtc_atomic_begin,
.atomic_flush = dpu_crtc_atomic_flush,
+ .mode_valid = dpu_crtc_mode_valid,
.get_scanout_position = dpu_crtc_get_scanout_position,
};
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 40e4b829b9da..1c86f22859fa 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1191,13 +1191,8 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
dev->mode_config.min_width = 0;
dev->mode_config.min_height = 0;
- /*
-* max crtc width is equal to the max mixer width * 2 and max height is
-* is 4K
-*/
- dev->mode_config.max_width =
- dpu_kms->catalog->caps->max_mixer_width * 2;
- dev->mode_config.max_height = 4096;
+ dev->mode_config.max_width = DPU_MAX_IMG_WIDTH;
+ dev->mode_config.max_height = DPU_MAX_IMG_HEIGHT;
dev->max_vblank_count = 0x;
/* Disable vblank irqs aggressively for power-saving */
--
2.39.2
[PATCH v5 11/16] drm/msm/dpu: split dpu_format_populate_layout
Split dpu_format_populate_layout() into addess-related and
pitch/format-related parts.
Reviewed-by: Abhinav Kumar
Tested-by: Abhinav Kumar # sc7280
Signed-off-by: Dmitry Baryshkov
---
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c| 8 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c| 32 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h| 8 --
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 15 --
4 files changed, 44 insertions(+), 19 deletions(-)
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 c4a16a73bc97..ede926d30285 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
@@ -579,7 +579,13 @@ static void dpu_encoder_phys_wb_prepare_wb_job(struct
dpu_encoder_phys *phys_enc
format = msm_framebuffer_format(job->fb);
- ret = dpu_format_populate_layout(aspace, job->fb, _cfg->dest);
+ ret = dpu_format_populate_plane_sizes(job->fb, _cfg->dest);
+ if (ret) {
+ DPU_DEBUG("failed to populate plane sizes%d\n", ret);
+ return;
+ }
+
+ ret = dpu_format_populate_addrs(aspace, job->fb, _cfg->dest);
if (ret) {
DPU_DEBUG("failed to populate layout %d\n", ret);
return;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
index 4d17eb88af40..abe3a1c0e409 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
@@ -93,7 +93,7 @@ static int _dpu_format_get_media_color_ubwc(const struct
msm_format *fmt)
return color_fmt;
}
-static int _dpu_format_get_plane_sizes_ubwc(
+static int _dpu_format_populate_plane_sizes_ubwc(
const struct msm_format *fmt,
struct drm_framebuffer *fb,
struct dpu_hw_fmt_layout *layout)
@@ -170,7 +170,7 @@ static int _dpu_format_get_plane_sizes_ubwc(
return 0;
}
-static int _dpu_format_get_plane_sizes_linear(
+static int _dpu_format_populate_plane_sizes_linear(
const struct msm_format *fmt,
struct drm_framebuffer *fb,
struct dpu_hw_fmt_layout *layout)
@@ -244,12 +244,21 @@ static int _dpu_format_get_plane_sizes_linear(
return 0;
}
-static int dpu_format_get_plane_sizes(
- const struct msm_format *fmt,
+/*
+ * dpu_format_populate_addrs - populate non-address part of the layout based on
+ * fb, and format found in the fb
+ * @fb:framebuffer pointer
+ * @layout: format layout structure to populate
+ *
+ * Return: error code on failure or 0 if new addresses were populated
+ */
+int dpu_format_populate_plane_sizes(
struct drm_framebuffer *fb,
struct dpu_hw_fmt_layout *layout)
{
- if (!layout || !fmt) {
+ const struct msm_format *fmt;
+
+ if (!layout || !fb) {
DRM_ERROR("invalid pointer\n");
return -EINVAL;
}
@@ -260,10 +269,12 @@ static int dpu_format_get_plane_sizes(
return -ERANGE;
}
+ fmt = msm_framebuffer_format(fb);
+
if (MSM_FORMAT_IS_UBWC(fmt) || MSM_FORMAT_IS_TILE(fmt))
- return _dpu_format_get_plane_sizes_ubwc(fmt, fb, layout);
+ return _dpu_format_populate_plane_sizes_ubwc(fmt, fb, layout);
- return _dpu_format_get_plane_sizes_linear(fmt, fb, layout);
+ return _dpu_format_populate_plane_sizes_linear(fmt, fb, layout);
}
static int _dpu_format_populate_addrs_ubwc(
@@ -377,7 +388,7 @@ static int _dpu_format_populate_addrs_linear(
return 0;
}
-int dpu_format_populate_layout(
+int dpu_format_populate_addrs(
struct msm_gem_address_space *aspace,
struct drm_framebuffer *fb,
struct dpu_hw_fmt_layout *layout)
@@ -398,11 +409,6 @@ int dpu_format_populate_layout(
fmt = msm_framebuffer_format(fb);
- /* Populate the plane sizes etc via get_format */
- ret = dpu_format_get_plane_sizes(fmt, fb, layout);
- if (ret)
- return ret;
-
/* Populate the addresses given the fb */
if (MSM_FORMAT_IS_UBWC(fmt) ||
MSM_FORMAT_IS_TILE(fmt))
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h
index ef1239c95058..2f2bff14c0db 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h
@@ -32,7 +32,7 @@ static inline bool dpu_find_format(u32 format, const u32
*supported_formats,
}
/**
- * dpu_format_populate_layout - populate the given format layout based on
+ * dpu_format_populate_addrs - populate buffer addresses based on
*
[PATCH v5 02/16] drm/msm/dpu: fix error condition in dpu_encoder_virt_atomic_mode_set
The commit b954fa6baaca ("drm/msm/dpu: Refactor rm iterator") removed
zero-init of the hw_ctl array, but didn't change the error condition,
that checked for hw_ctl[i] being NULL. Use indices check instead.
Fixes: b954fa6baaca ("drm/msm/dpu: Refactor rm iterator")
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 5d205e09cf45..7613005fbfea 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1186,7 +1186,7 @@ static void dpu_encoder_virt_atomic_mode_set(struct
drm_encoder *drm_enc,
return;
}
- if (!hw_ctl[i]) {
+ if (i >= num_ctl) {
DPU_ERROR_ENC(dpu_enc,
"no ctl block assigned at idx: %d\n", i);
return;
--
2.39.2
[PATCH v5 06/16] drm/msm/dpu: drop dpu_format_populate_layout from dpu_plane_sspp_atomic_update
The dpu_plane_prepare_fb() already calls dpu_format_populate_layout().
Store the generated layout in the plane state and drop this call from
dpu_plane_sspp_update().
Reviewed-by: Abhinav Kumar
Tested-by: Abhinav Kumar # sc7280
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 19 ---
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 3 +++
2 files changed, 7 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index eabc4813c649..241c2d7a218a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -647,7 +647,6 @@ static int dpu_plane_prepare_fb(struct drm_plane *plane,
struct drm_framebuffer *fb = new_state->fb;
struct dpu_plane *pdpu = to_dpu_plane(plane);
struct dpu_plane_state *pstate = to_dpu_plane_state(new_state);
- struct dpu_hw_fmt_layout layout;
struct dpu_kms *kms = _dpu_plane_get_kms(>base);
int ret;
@@ -677,7 +676,8 @@ static int dpu_plane_prepare_fb(struct drm_plane *plane,
/* validate framebuffer layout before commit */
ret = dpu_format_populate_layout(pstate->aspace,
- new_state->fb, );
+new_state->fb,
+>layout);
if (ret) {
DPU_ERROR_PLANE(pdpu, "failed to get format layout, %d\n", ret);
if (pstate->aspace)
@@ -1103,17 +1103,6 @@ static void dpu_plane_sspp_atomic_update(struct
drm_plane *plane)
msm_framebuffer_format(fb);
struct dpu_sw_pipe_cfg *pipe_cfg = >pipe_cfg;
struct dpu_sw_pipe_cfg *r_pipe_cfg = >r_pipe_cfg;
- struct dpu_kms *kms = _dpu_plane_get_kms(>base);
- struct msm_gem_address_space *aspace = kms->base.aspace;
- struct dpu_hw_fmt_layout layout;
- bool layout_valid = false;
- int ret;
-
- ret = dpu_format_populate_layout(aspace, fb, );
- if (ret)
- DPU_ERROR_PLANE(pdpu, "failed to get format layout, %d\n", ret);
- else
- layout_valid = true;
pstate->pending = true;
@@ -1128,12 +1117,12 @@ static void dpu_plane_sspp_atomic_update(struct
drm_plane *plane)
dpu_plane_sspp_update_pipe(plane, pipe, pipe_cfg, fmt,
drm_mode_vrefresh(>mode),
- layout_valid ? : NULL);
+ >layout);
if (r_pipe->sspp) {
dpu_plane_sspp_update_pipe(plane, r_pipe, r_pipe_cfg, fmt,
drm_mode_vrefresh(>mode),
- layout_valid ? : NULL);
+ >layout);
}
if (pstate->needs_qos_remap)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
index abd6b21a049b..348b0075d1ce 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h
@@ -31,6 +31,7 @@
* @plane_clk: calculated clk per plane
* @needs_dirtyfb: whether attached CRTC needs pixel data explicitly flushed
* @rotation: simplified drm rotation hint
+ * @layout: framebuffer memory layout
*/
struct dpu_plane_state {
struct drm_plane_state base;
@@ -48,6 +49,8 @@ struct dpu_plane_state {
bool needs_dirtyfb;
unsigned int rotation;
+
+ struct dpu_hw_fmt_layout layout;
};
#define to_dpu_plane_state(x) \
--
2.39.2
[PATCH v5 12/16] drm/msm/dpu: make dpu_format_populate_addrs return void
The function msm_framebuffer_iova() can not fail, it always returns a
valid address. Drop the useless checks (that were already performed at
the time) and make dpu_format_populate_addrs() return void.
Signed-off-by: Dmitry Baryshkov
---
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c| 6 +--
drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c| 62 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h| 10 ++--
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 14 ++---
4 files changed, 21 insertions(+), 71 deletions(-)
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 ede926d30285..30d87ff3c227 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
@@ -585,11 +585,7 @@ static void dpu_encoder_phys_wb_prepare_wb_job(struct
dpu_encoder_phys *phys_enc
return;
}
- ret = dpu_format_populate_addrs(aspace, job->fb, _cfg->dest);
- if (ret) {
- DPU_DEBUG("failed to populate layout %d\n", ret);
- return;
- }
+ dpu_format_populate_addrs(aspace, job->fb, _cfg->dest);
wb_cfg->dest.width = job->fb->width;
wb_cfg->dest.height = job->fb->height;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
index abe3a1c0e409..095bb947f1ff 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c
@@ -277,25 +277,15 @@ int dpu_format_populate_plane_sizes(
return _dpu_format_populate_plane_sizes_linear(fmt, fb, layout);
}
-static int _dpu_format_populate_addrs_ubwc(
- struct msm_gem_address_space *aspace,
- struct drm_framebuffer *fb,
- struct dpu_hw_fmt_layout *layout)
+static void _dpu_format_populate_addrs_ubwc(struct msm_gem_address_space
*aspace,
+ struct drm_framebuffer *fb,
+ struct dpu_hw_fmt_layout *layout)
{
const struct msm_format *fmt;
uint32_t base_addr = 0;
bool meta;
- if (!fb || !layout) {
- DRM_ERROR("invalid pointers\n");
- return -EINVAL;
- }
-
base_addr = msm_framebuffer_iova(fb, aspace, 0);
- if (!base_addr) {
- DRM_ERROR("failed to retrieve base addr\n");
- return -EFAULT;
- }
fmt = msm_framebuffer_format(fb);
meta = MSM_FORMAT_IS_UBWC(fmt);
@@ -330,7 +320,7 @@ static int _dpu_format_populate_addrs_ubwc(
+ layout->plane_size[2] + layout->plane_size[3];
if (!meta)
- return 0;
+ return;
/* configure Y metadata plane */
layout->plane_addr[2] = base_addr;
@@ -361,60 +351,36 @@ static int _dpu_format_populate_addrs_ubwc(
layout->plane_addr[1] = 0;
if (!meta)
- return 0;
+ return;
layout->plane_addr[2] = base_addr;
layout->plane_addr[3] = 0;
}
- return 0;
}
-static int _dpu_format_populate_addrs_linear(
- struct msm_gem_address_space *aspace,
- struct drm_framebuffer *fb,
- struct dpu_hw_fmt_layout *layout)
+static void _dpu_format_populate_addrs_linear(struct msm_gem_address_space
*aspace,
+ struct drm_framebuffer *fb,
+ struct dpu_hw_fmt_layout *layout)
{
unsigned int i;
/* Populate addresses for simple formats here */
- for (i = 0; i < layout->num_planes; ++i) {
+ for (i = 0; i < layout->num_planes; ++i)
layout->plane_addr[i] = msm_framebuffer_iova(fb, aspace, i);
- if (!layout->plane_addr[i]) {
- DRM_ERROR("failed to retrieve base addr\n");
- return -EFAULT;
- }
- }
-
- return 0;
}
-int dpu_format_populate_addrs(
- struct msm_gem_address_space *aspace,
- struct drm_framebuffer *fb,
- struct dpu_hw_fmt_layout *layout)
+void dpu_format_populate_addrs(struct msm_gem_address_space *aspace,
+ struct drm_framebuffer *fb,
+ struct dpu_hw_fmt_layout *layout)
{
const struct msm_format *fmt;
- int ret;
-
- if (!fb || !layout) {
- DRM_ERROR("invalid arguments\n");
- return -EINVAL;
- }
-
- if ((fb->width > DPU_MAX_IMG_WIDTH) ||
- (fb->height > DPU_MAX_IMG_HEIGH
[PATCH v5 04/16] drm/msm/dpu: check for overflow in _dpu_crtc_setup_lm_bounds()
Make _dpu_crtc_setup_lm_bounds() check that CRTC width is not
overflowing LM requirements. Rename the function accordingly.
Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support")
Reviewed-by: Abhinav Kumar
Tested-by: Abhinav Kumar # sc7280
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 7399794d75eb..2a87dd7188b8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -711,12 +711,13 @@ void dpu_crtc_complete_commit(struct drm_crtc *crtc)
_dpu_crtc_complete_flip(crtc);
}
-static void _dpu_crtc_setup_lm_bounds(struct drm_crtc *crtc,
+static int _dpu_crtc_check_and_setup_lm_bounds(struct drm_crtc *crtc,
struct drm_crtc_state *state)
{
struct dpu_crtc_state *cstate = to_dpu_crtc_state(state);
struct drm_display_mode *adj_mode = >adjusted_mode;
u32 crtc_split_width = adj_mode->hdisplay / cstate->num_mixers;
+ struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
int i;
for (i = 0; i < cstate->num_mixers; i++) {
@@ -727,7 +728,12 @@ static void _dpu_crtc_setup_lm_bounds(struct drm_crtc
*crtc,
r->y2 = adj_mode->vdisplay;
trace_dpu_crtc_setup_lm_bounds(DRMID(crtc), i, r);
+
+ if (drm_rect_width(r) > dpu_kms->catalog->caps->max_mixer_width)
+ return -E2BIG;
}
+
+ return 0;
}
static void _dpu_crtc_get_pcc_coeff(struct drm_crtc_state *state,
@@ -803,7 +809,7 @@ static void dpu_crtc_atomic_begin(struct drm_crtc *crtc,
DRM_DEBUG_ATOMIC("crtc%d\n", crtc->base.id);
- _dpu_crtc_setup_lm_bounds(crtc, crtc->state);
+ _dpu_crtc_check_and_setup_lm_bounds(crtc, crtc->state);
/* encoder will trigger pending mask now */
drm_for_each_encoder_mask(encoder, crtc->dev, crtc->state->encoder_mask)
@@ -1194,8 +1200,11 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
if (crtc_state->active_changed)
crtc_state->mode_changed = true;
- if (cstate->num_mixers)
- _dpu_crtc_setup_lm_bounds(crtc, crtc_state);
+ if (cstate->num_mixers) {
+ rc = _dpu_crtc_check_and_setup_lm_bounds(crtc, crtc_state);
+ if (rc)
+ return rc;
+ }
/* FIXME: move this to dpu_plane_atomic_check? */
drm_atomic_crtc_state_for_each_plane_state(plane, pstate, crtc_state) {
--
2.39.2
