[PATCH v2 03/27] drm/msm/dpu: move SSPP allocation to the RM
Abhinav Kumar
quic_abhinavk at quicinc.com
Fri Jan 27 22:59:35 UTC 2023
On 1/26/2023 9:56 PM, Dmitry Baryshkov wrote:
> On Fri, 27 Jan 2023 at 01:29, Abhinav Kumar <quic_abhinavk at quicinc.com> wrote:
>>
>>
>>
>> On 12/29/2022 11:18 AM, Dmitry Baryshkov wrote:
>>> Follow the example of all other hw blocks and initialize SSPP blocks in
>>> Resource Manager.
>>>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>>> ---
>>> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 17 ++++-------------
>>> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 22 ++++++++++++++++++++++
>>> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h | 12 ++++++++++++
>>> 3 files changed, 38 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>>> index e1cdd71716f0..e443799de2c1 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>>> @@ -1275,8 +1275,6 @@ static void dpu_plane_destroy(struct drm_plane *plane)
>>> /* this will destroy the states as well */
>>> drm_plane_cleanup(plane);
>>>
>>> - dpu_hw_sspp_destroy(pdpu->pipe_hw);
>>> -
>> We removed from here so the flow will be msm_drm_uninit calls
>> drm_mode_config_cleanup() which will call kms->destroy() which shall
>> call dpu_rm_destroy() where this will be released now right?
>
> Yes. _dpu_kms_hw_destroy() calls dpu_rm_destroy(), which destroys all
> RM-allocated objects.
>
Ack, thanks
>>
>>
>>> kfree(pdpu);
>>> }
>>> }
>>> @@ -1482,14 +1480,10 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
>>> pdpu->pipe = pipe;
>>>
>>> /* initialize underlying h/w driver */
>>> - pdpu->pipe_hw = dpu_hw_sspp_init(pipe, kms->mmio, kms->catalog);
>>> - if (IS_ERR(pdpu->pipe_hw)) {
>>> - DPU_ERROR("[%u]SSPP init failed\n", pipe);
>>> - ret = PTR_ERR(pdpu->pipe_hw);
>>> + pdpu->pipe_hw = dpu_rm_get_sspp(&kms->rm, pipe);
>>> + if (!pdpu->pipe_hw || !pdpu->pipe_hw->cap || !pdpu->pipe_hw->cap->sblk) {
>>> + DPU_ERROR("[%u]SSPP is invalid\n", pipe);
>>
>> I know this was existing code but can there be a case where pipe_hw->cap
>> exists but pipe_hw->cap->sblk doesnt?
>
> There is none up to now, granted that SSPP has a lot of extra data in
> sblk. However, as you said, it's an existing code. No need to change
> it here. And it's good as a safety net.
> Granted that SRC offset is always 0, we can safely drop the src_sblk
> and always access it via main register space.
>
Alright, since this is existing code, lets have it as a separate change.
Hence,
Reviewed-by: Abhinav Kumar <quic_abhinavk at quicinc.com>
>>
>>> goto clean_plane;
>>> - } else if (!pdpu->pipe_hw->cap || !pdpu->pipe_hw->cap->sblk) {
>>> - DPU_ERROR("[%u]SSPP init returned invalid cfg\n", pipe);
>>> - goto clean_sspp;
>>> }
>>>
>>> format_list = pdpu->pipe_hw->cap->sblk->format_list;
>>> @@ -1499,7 +1493,7 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
>>> format_list, num_formats,
>>> supported_format_modifiers, type, NULL);
>>> if (ret)
>>> - goto clean_sspp;
>>> + goto clean_plane;
>>>
>>> pdpu->catalog = kms->catalog;
>>>
>>> @@ -1532,9 +1526,6 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev,
>>> pipe, plane->base.id);
>>> return plane;
>>>
>>> -clean_sspp:
>>> - if (pdpu && pdpu->pipe_hw)
>>> - dpu_hw_sspp_destroy(pdpu->pipe_hw);
>>> clean_plane:
>>> kfree(pdpu);
>>> return ERR_PTR(ret);
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
>>> index 73b3442e7467..0668009cc9ed 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
>>> @@ -8,6 +8,7 @@
>>> #include "dpu_hw_lm.h"
>>> #include "dpu_hw_ctl.h"
>>> #include "dpu_hw_pingpong.h"
>>> +#include "dpu_hw_sspp.h"
>>> #include "dpu_hw_intf.h"
>>> #include "dpu_hw_wb.h"
>>> #include "dpu_hw_dspp.h"
>>> @@ -91,6 +92,9 @@ int dpu_rm_destroy(struct dpu_rm *rm)
>>> for (i = 0; i < ARRAY_SIZE(rm->hw_wb); i++)
>>> dpu_hw_wb_destroy(rm->hw_wb[i]);
>>>
>>> + for (i = 0; i < ARRAY_SIZE(rm->hw_sspp); i++)
>>> + dpu_hw_sspp_destroy(rm->hw_sspp[i]);
>>> +
>>> return 0;
>>> }
>>>
>>> @@ -255,6 +259,24 @@ int dpu_rm_init(struct dpu_rm *rm,
>>> rm->dsc_blks[dsc->id - DSC_0] = &hw->base;
>>> }
>>>
>>> + for (i = 0; i < cat->sspp_count; i++) {
>>> + struct dpu_hw_sspp *hw;
>>> + const struct dpu_sspp_cfg *sspp = &cat->sspp[i];
>>> +
>>> + if (sspp->id < SSPP_NONE || sspp->id >= SSPP_MAX) {
>>> + DPU_ERROR("skip intf %d with invalid id\n", sspp->id);
>>> + continue;
>>> + }
>>> +
>>> + hw = dpu_hw_sspp_init(sspp->id, mmio, cat);
>>> + if (IS_ERR(hw)) {
>>> + rc = PTR_ERR(hw);
>>> + DPU_ERROR("failed sspp object creation: err %d\n", rc);
>>> + goto fail;
>>> + }
>>> + rm->hw_sspp[sspp->id - SSPP_NONE] = hw;
>>> + }
>>> +
>>> return 0;
>>>
>>> fail:
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
>>> index 59de72b381f9..d62c2edb2460 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.h
>>> @@ -21,6 +21,7 @@ struct dpu_global_state;
>>> * @hw_intf: array of intf hardware resources
>>> * @hw_wb: array of wb hardware resources
>>> * @dspp_blks: array of dspp hardware resources
>>> + * @hw_sspp: array of sspp hardware resources
>>> */
>>> struct dpu_rm {
>>> struct dpu_hw_blk *pingpong_blks[PINGPONG_MAX - PINGPONG_0];
>>> @@ -31,6 +32,7 @@ struct dpu_rm {
>>> struct dpu_hw_blk *dspp_blks[DSPP_MAX - DSPP_0];
>>> struct dpu_hw_blk *merge_3d_blks[MERGE_3D_MAX - MERGE_3D_0];
>>> struct dpu_hw_blk *dsc_blks[DSC_MAX - DSC_0];
>>> + struct dpu_hw_sspp *hw_sspp[SSPP_MAX - SSPP_NONE];
>>> };
>>>
>>> /**
>>> @@ -108,5 +110,15 @@ static inline struct dpu_hw_wb *dpu_rm_get_wb(struct dpu_rm *rm, enum dpu_wb wb_
>>> return rm->hw_wb[wb_idx - WB_0];
>>> }
>>>
>>> +/**
>>> + * dpu_rm_get_sspp - Return a struct dpu_hw_sspp instance given it's index.
>>> + * @rm: DPU Resource Manager handle
>>> + * @sspp_idx: SSPP index
>>> + */
>>> +static inline struct dpu_hw_sspp *dpu_rm_get_sspp(struct dpu_rm *rm, enum dpu_sspp sspp_idx)
>>> +{
>>> + return rm->hw_sspp[sspp_idx - SSPP_NONE];
>>> +}
>>> +
>>> #endif /* __DPU_RM_H__ */
>>>
>
>
>
More information about the dri-devel
mailing list