diff mbox series

[v2,03/27] drm/msm/dpu: move SSPP allocation to the RM

Message ID 20221229191856.3508092-4-dmitry.baryshkov@linaro.org (mailing list archive)
State Superseded
Headers show
Series drm/msm/dpu: wide planes support | expand

Commit Message

Dmitry Baryshkov Dec. 29, 2022, 7:18 p.m. UTC
Follow the example of all other hw blocks and initialize SSPP blocks in
Resource Manager.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@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(-)

Comments

Abhinav Kumar Jan. 26, 2023, 11:29 p.m. UTC | #1
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@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?


>   		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?

>   		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__ */
>
Dmitry Baryshkov Jan. 27, 2023, 5:56 a.m. UTC | #2
On Fri, 27 Jan 2023 at 01:29, Abhinav Kumar <quic_abhinavk@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@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.

>
>
> >               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.

>
> >               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__ */
> >
Abhinav Kumar Jan. 27, 2023, 10:59 p.m. UTC | #3
On 1/26/2023 9:56 PM, Dmitry Baryshkov wrote:
> On Fri, 27 Jan 2023 at 01:29, Abhinav Kumar <quic_abhinavk@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@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@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__ */
>>>
> 
> 
>
diff mbox series

Patch

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);
-
 		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);
 		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__ */