Re: [Freedreno] [PATCH -next] drm/msm: dpu: Fix "WARNING: invalid free of devm_ allocated data"
On Fri, Nov 16, 2018 at 07:25:26PM +0800, YueHaibing wrote: > 'dpu_enc' is a member of 'drm_enc' It's the other way around :) > And 'drm_enc' got allocated with devm_kzalloc in dpu_encoder_init. > > This gives this error message: > ./drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c:459:1-6: > WARNING: invalid free of devm_ allocated data > > Signed-off-by: YueHaibing Reviewed-by: Sean Paul > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > index 82c55ef..99526d9 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -455,8 +455,6 @@ static void dpu_encoder_destroy(struct drm_encoder > *drm_enc) > > drm_encoder_cleanup(drm_enc); > mutex_destroy(_enc->enc_lock); > - > - kfree(dpu_enc); > } > > void dpu_encoder_helper_split_config( > -- > 2.7.0 > > -- Sean Paul, Software Engineer, Google / Chromium OS ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 1/2] drm/msm/dpu: add dpu encoder uninit
On Fri, Nov 16, 2018 at 04:35:26PM -0500, Sean Paul wrote:
> On Fri, Nov 16, 2018 at 11:22:21AM -0800, Jeykumar Sankaran wrote:
> > Add encoder interface to release dpu encoder
> > on mode_init failures in kms.
> >
> > Signed-off-by: Jeykumar Sankaran
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 12 ++--
> > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 6 ++
> > 2 files changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > index dd7ab85..b253165 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > @@ -422,6 +422,15 @@ void dpu_encoder_get_hw_resources(struct drm_encoder
> > *drm_enc,
> > }
> > }
> >
> > +void dpu_encoder_uninit(struct drm_encoder *drm_enc)
> > +{
> > + struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
> > +
> > + drm_encoder_cleanup(drm_enc);
> > +
> > + kfree(dpu_enc);
> > +}
> > +
> > static void dpu_encoder_destroy(struct drm_encoder *drm_enc)
> > {
> > struct dpu_encoder_virt *dpu_enc = NULL;
> > @@ -453,10 +462,9 @@ static void dpu_encoder_destroy(struct drm_encoder
> > *drm_enc)
> > dpu_enc->num_phys_encs = 0;
> > mutex_unlock(_enc->enc_lock);
> >
> > - drm_encoder_cleanup(drm_enc);
> > mutex_destroy(_enc->enc_lock);
> >
> > - kfree(dpu_enc);
> > + dpu_encoder_uninit(drm_enc);
> > }
> >
> > void dpu_encoder_helper_split_config(
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> > index 9dbf38f..60b88bd 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> > @@ -142,6 +142,12 @@ struct drm_encoder *dpu_encoder_init(
> > int drm_enc_mode);
> >
> > /**
> > + * dpu_encoder_uninit - uninitialize virtual encoder object
> > + * @drm_enc: Pointer to drm encoder
> > + */
> > +void dpu_encoder_uninit(struct drm_encoder *drm_enc);
>
> Just make it static?
I just saw YueHaibing's patch to remove the kfree entirely since dpu_enc is
devm_* managed. IMO, that'd be a better solution than this.
Sean
>
> > +
> > +/**
> > * dpu_encoder_setup - setup dpu_encoder for the display probed
> > * @dev: Pointer to drm device structure
> > * @enc: Pointer to the drm_encoder
> > --
> > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> > a Linux Foundation Collaborative Project
> >
>
> --
> Sean Paul, Software Engineer, Google / Chromium OS
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 1/2] drm/msm/dpu: add dpu encoder uninit
On Fri, Nov 16, 2018 at 11:22:21AM -0800, Jeykumar Sankaran wrote:
> Add encoder interface to release dpu encoder
> on mode_init failures in kms.
>
> Signed-off-by: Jeykumar Sankaran
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 12 ++--
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 6 ++
> 2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index dd7ab85..b253165 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -422,6 +422,15 @@ void dpu_encoder_get_hw_resources(struct drm_encoder
> *drm_enc,
> }
> }
>
> +void dpu_encoder_uninit(struct drm_encoder *drm_enc)
> +{
> + struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
> +
> + drm_encoder_cleanup(drm_enc);
> +
> + kfree(dpu_enc);
> +}
> +
> static void dpu_encoder_destroy(struct drm_encoder *drm_enc)
> {
> struct dpu_encoder_virt *dpu_enc = NULL;
> @@ -453,10 +462,9 @@ static void dpu_encoder_destroy(struct drm_encoder
> *drm_enc)
> dpu_enc->num_phys_encs = 0;
> mutex_unlock(_enc->enc_lock);
>
> - drm_encoder_cleanup(drm_enc);
> mutex_destroy(_enc->enc_lock);
>
> - kfree(dpu_enc);
> + dpu_encoder_uninit(drm_enc);
> }
>
> void dpu_encoder_helper_split_config(
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> index 9dbf38f..60b88bd 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> @@ -142,6 +142,12 @@ struct drm_encoder *dpu_encoder_init(
> int drm_enc_mode);
>
> /**
> + * dpu_encoder_uninit - uninitialize virtual encoder object
> + * @drm_enc: Pointer to drm encoder
> + */
> +void dpu_encoder_uninit(struct drm_encoder *drm_enc);
Just make it static?
> +
> +/**
> * dpu_encoder_setup - setup dpu_encoder for the display probed
> * @dev: Pointer to drm device structure
> * @enc: Pointer to the drm_encoder
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v2 15/24] drm/msm: dpu: Stop using encoder->crtc pointer
On Fri, Nov 16, 2018 at 12:05:09PM -0800, Jeykumar Sankaran wrote:
> On 2018-11-16 10:42, Sean Paul wrote:
> > From: Sean Paul
> >
> > It's for legacy drivers, for atomic drivers crtc->state->encoder_mask
> > should be used to map encoder to crtc.
> >
> > Changes in v2:
> > - None
> >
> > Signed-off-by: Sean Paul
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 46
> > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 19 +++---
> > 2 files changed, 29 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > index 156f4c77ca44..a008a87a8113 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > @@ -284,9 +284,9 @@ enum dpu_intf_mode dpu_crtc_get_intf_mode(struct
> > drm_crtc *crtc)
> > return INTF_MODE_NONE;
> > }
> >
> > - drm_for_each_encoder(encoder, crtc->dev)
> > - if (encoder->crtc == crtc)
> > - return dpu_encoder_get_intf_mode(encoder);
> > + /* TODO: Returns the first INTF_MODE, could there be multiple
> > values? */
> > + drm_for_each_encoder_mask(encoder, crtc->dev,
> > crtc->state->encoder_mask)
> > + return dpu_encoder_get_intf_mode(encoder);
> >
> > return INTF_MODE_NONE;
> > }
> > @@ -551,13 +551,9 @@ static void dpu_crtc_atomic_begin(struct drm_crtc
> > *crtc,
> > spin_unlock_irqrestore(>event_lock, flags);
> > }
> >
> > - list_for_each_entry(encoder, >mode_config.encoder_list, head)
> > {
> > - if (encoder->crtc != crtc)
> > - continue;
> > -
> > - /* encoder will trigger pending mask now */
> > + /* encoder will trigger pending mask now */
> > + drm_for_each_encoder_mask(encoder, crtc->dev,
> > crtc->state->encoder_mask)
> > dpu_encoder_trigger_kickoff_pending(encoder);
> > - }
> >
> > /*
> > * If no mixers have been allocated in dpu_crtc_atomic_check(),
> > @@ -704,7 +700,6 @@ static int _dpu_crtc_wait_for_frame_done(struct
> > drm_crtc *crtc)
> > void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
> > {
> > struct drm_encoder *encoder;
> > - struct drm_device *dev = crtc->dev;
> > struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
> > struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
> > struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state);
> > @@ -720,16 +715,13 @@ void dpu_crtc_commit_kickoff(struct drm_crtc
> > *crtc)
> >
> > DPU_ATRACE_BEGIN("crtc_commit");
> >
> > - list_for_each_entry(encoder, >mode_config.encoder_list, head)
> > {
> > + /*
> > +* Encoder will flush/start now, unless it has a tx pending. If
> > so, it
> > +* may delay and flush at an irq event (e.g. ppdone)
> > +*/
> > + drm_for_each_encoder_mask(encoder, crtc->dev,
> > + crtc->state->encoder_mask) {
> > struct dpu_encoder_kickoff_params params = { 0 };
> > -
> > - if (encoder->crtc != crtc)
> > - continue;
> > -
> > - /*
> > -* Encoder will flush/start now, unless it has a tx
> > pending.
> > -* If so, it may delay and flush at an irq event (e.g.
> > ppdone)
> > -*/
> > dpu_encoder_prepare_for_kickoff(encoder, );
> > }
> >
> > @@ -754,12 +746,8 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
> >
> > dpu_vbif_clear_errors(dpu_kms);
> >
> > - list_for_each_entry(encoder, >mode_config.encoder_list, head)
> > {
> > - if (encoder->crtc != crtc)
> > - continue;
> > -
> > + drm_for_each_encoder_mask(encoder, crtc->dev,
> > crtc->state->encoder_mask)
> > dpu_encoder_kickoff(encoder);
> > - }
> We wont be holding the modeset locks here (and in crtc_atomic_begin) in the
> display thread. Is
> it safe to iterate over encoder_mask?
Hmm, I'm not sure I follow. AFAICT, there are 2 callsites for
dpu_crtc_commit_kickoff():
1- dpu_kms_encoder_enable() which is called via the encoder->funcs->enable hook
2- dpu_kms_commit() which is called in the
mode_config->funcs->atomic_commit_tail
Both of these callsites will hold the modeset locks.
Am I missing something?
Thanks for your review,
Sean
> >
> > end:
> > reinit_completion(_crtc->frame_done_comp);
> > @@ -883,11 +871,8 @@ static void dpu_crtc_disable(struct drm_crtc *crtc)
> >
> > dpu_core_perf_crtc_update(crtc, 0, true);
> >
> > - drm_for_each_encoder(encoder, crtc->dev) {
> > - if (encoder->crtc != crtc)
> > - continue;
> > + 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;
> > @@ -922,12 +907,9 @@ static void dpu_crtc_enable(struct drm_crtc *crtc,
> >
Re: [Freedreno] [PATCH 2/2] drm/msm/dpu: add display port support in DPU
On Fri, Nov 16, 2018 at 11:22:22AM -0800, Jeykumar Sankaran wrote:
> Add display port support in DPU by creating hooks
> for DP encoder enumeration and encoder mode
> initialization.
>
> This change is based on the SDM845 Display port
> driver changes[1].
>
> [1] https://lwn.net/Articles/768265/
>
> Signed-off-by: Jeykumar Sankaran
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 3 ++
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 49
> +
> 2 files changed, 46 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index b253165..e9c7edc6 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -2048,6 +2048,9 @@ static int dpu_encoder_setup_display(struct
> dpu_encoder_virt *dpu_enc,
> case DRM_MODE_ENCODER_DSI:
> intf_type = INTF_DSI;
> break;
> + case DRM_MODE_ENCODER_TMDS:
> + intf_type = INTF_DP;
> + break;
> default:
> DPU_ERROR_ENC(dpu_enc, "unsupported display interface type\n");
> return -EINVAL;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 985c855..b823a37 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -473,6 +473,31 @@ static void _dpu_kms_initialize_dsi(struct drm_device
> *dev,
> }
> }
>
> +static void _dpu_kms_initialize_displayport(struct drm_device *dev,
> + struct msm_drm_private *priv,
> + struct dpu_kms *dpu_kms)
> +{
> + struct drm_encoder *encoder = NULL;
> + int rc;
> +
> + if (!priv->dp)
> + return;
> +
> + encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_TMDS);
> + if (IS_ERR_OR_NULL(encoder)) {
dpu_encoder_init() only returns valid pointer or ERR_PTR() so a Only IS_ERR() is
needed.
> + DPU_ERROR("encoder init failed for dsi display\n");
> + return;
> + }
> +
> + priv->encoders[priv->num_encoders++] = encoder;
> +
> + rc = msm_dp_modeset_init(priv->dp, dev, encoder);
> + if (rc) {
> + DPU_ERROR("modeset_init failed for DP, rc = %d\n", rc);
> + dpu_encoder_uninit(encoder);
> + }
> +}
> +
> /**
> * _dpu_kms_setup_displays - create encoders, bridges and connectors
> * for underlying displays
> @@ -487,6 +512,8 @@ static void _dpu_kms_setup_displays(struct drm_device
> *dev,
> {
> _dpu_kms_initialize_dsi(dev, priv, dpu_kms);
>
> + _dpu_kms_initialize_displayport(dev, priv, dpu_kms);
> +
> /**
>* Extend this function to initialize other
>* types of displays
> @@ -723,13 +750,23 @@ static void _dpu_kms_set_encoder_mode(struct msm_kms
> *kms,
> info.capabilities = cmd_mode ? MSM_DISPLAY_CAP_CMD_MODE :
> MSM_DISPLAY_CAP_VID_MODE;
>
> - /* TODO: No support for DSI swap */
> - for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
> - if (priv->dsi[i]) {
> - info.h_tile_instance[info.num_of_h_tiles] = i;
> - info.num_of_h_tiles++;
> + switch (info.intf_type) {
> + case DRM_MODE_ENCODER_DSI:
> + /* TODO: No support for DSI swap */
> + for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
> + if (priv->dsi[i]) {
> + info.h_tile_instance[info.num_of_h_tiles] = i;
> + info.num_of_h_tiles++;
> + }
> }
> - }
> + break;
> + case DRM_MODE_ENCODER_TMDS:
> + info.num_of_h_tiles = 1;
> + break;
> + default:
> + DPU_ERROR("Invalid connector type\n");
> + return;
> + };
This is still going to be one of two options, a simple if/else can suffice here
and you don't need a log message and a default path.
>
> rc = dpu_encoder_setup(encoder->dev, encoder, );
> if (rc)
Jordan
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH -next] drm/msm: dpu: Fix "WARNING: invalid free of devm_ allocated data"
On Fri, Nov 16, 2018 at 07:25:26PM +0800, YueHaibing wrote: > 'dpu_enc' is a member of 'drm_enc' > And 'drm_enc' got allocated with devm_kzalloc in dpu_encoder_init. > > This gives this error message: > ./drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c:459:1-6: > WARNING: invalid free of devm_ allocated data I had partial fix of this in my ill-fated patch from a few weeks ago but this is better. Reviewed-by: Jordan Crouse > Signed-off-by: YueHaibing > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > index 82c55ef..99526d9 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c > @@ -455,8 +455,6 @@ static void dpu_encoder_destroy(struct drm_encoder > *drm_enc) > > drm_encoder_cleanup(drm_enc); > mutex_destroy(_enc->enc_lock); > - > - kfree(dpu_enc); > } > > void dpu_encoder_helper_split_config( > -- > 2.7.0 > > -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v2 16/24] drm/msm: dpu: Add modeset lock checks where applicable
On 2018-11-16 10:42, Sean Paul wrote:
From: Sean Paul
Add modeset lock checks to functions that could be called outside the
core atomic stack.
Changes in v2:
- None
Signed-off-by: Sean Paul
---
Reviewed-by: Jeykumar Sankaran
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 +
2 files changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index a008a87a8113..cd0a0bea4335 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -284,6 +284,8 @@ enum dpu_intf_mode dpu_crtc_get_intf_mode(struct
drm_crtc *crtc)
return INTF_MODE_NONE;
}
+ WARN_ON(!drm_modeset_is_locked(>mutex));
+
/* TODO: Returns the first INTF_MODE, could there be multiple
values? */
drm_for_each_encoder_mask(encoder, crtc->dev,
crtc->state->encoder_mask)
return dpu_encoder_get_intf_mode(encoder);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 64134d619748..5104fc01147e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -358,6 +358,7 @@ void dpu_kms_encoder_enable(struct drm_encoder
*encoder)
if (funcs && funcs->commit)
funcs->commit(encoder);
+
WARN_ON(!drm_modeset_is_locked(>mode_config.connection_mutex));
drm_for_each_crtc(crtc, dev) {
if (!(crtc->state->encoder_mask &
drm_encoder_mask(encoder)))
continue;
--
Jeykumar S
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v2 15/24] drm/msm: dpu: Stop using encoder->crtc pointer
On 2018-11-16 10:42, Sean Paul wrote:
From: Sean Paul
It's for legacy drivers, for atomic drivers crtc->state->encoder_mask
should be used to map encoder to crtc.
Changes in v2:
- None
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 46
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 19 +++---
2 files changed, 29 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 156f4c77ca44..a008a87a8113 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -284,9 +284,9 @@ enum dpu_intf_mode dpu_crtc_get_intf_mode(struct
drm_crtc *crtc)
return INTF_MODE_NONE;
}
- drm_for_each_encoder(encoder, crtc->dev)
- if (encoder->crtc == crtc)
- return dpu_encoder_get_intf_mode(encoder);
+ /* TODO: Returns the first INTF_MODE, could there be multiple
values? */
+ drm_for_each_encoder_mask(encoder, crtc->dev,
crtc->state->encoder_mask)
+ return dpu_encoder_get_intf_mode(encoder);
return INTF_MODE_NONE;
}
@@ -551,13 +551,9 @@ static void dpu_crtc_atomic_begin(struct drm_crtc
*crtc,
spin_unlock_irqrestore(>event_lock, flags);
}
- list_for_each_entry(encoder, >mode_config.encoder_list, head)
{
- if (encoder->crtc != crtc)
- continue;
-
- /* encoder will trigger pending mask now */
+ /* encoder will trigger pending mask now */
+ drm_for_each_encoder_mask(encoder, crtc->dev,
crtc->state->encoder_mask)
dpu_encoder_trigger_kickoff_pending(encoder);
- }
/*
* If no mixers have been allocated in dpu_crtc_atomic_check(),
@@ -704,7 +700,6 @@ static int _dpu_crtc_wait_for_frame_done(struct
drm_crtc *crtc)
void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
{
struct drm_encoder *encoder;
- struct drm_device *dev = crtc->dev;
struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state);
@@ -720,16 +715,13 @@ void dpu_crtc_commit_kickoff(struct drm_crtc
*crtc)
DPU_ATRACE_BEGIN("crtc_commit");
- list_for_each_entry(encoder, >mode_config.encoder_list, head)
{
+ /*
+* Encoder will flush/start now, unless it has a tx pending. If
so, it
+* may delay and flush at an irq event (e.g. ppdone)
+*/
+ drm_for_each_encoder_mask(encoder, crtc->dev,
+ crtc->state->encoder_mask) {
struct dpu_encoder_kickoff_params params = { 0 };
-
- if (encoder->crtc != crtc)
- continue;
-
- /*
-* Encoder will flush/start now, unless it has a tx
pending.
-* If so, it may delay and flush at an irq event (e.g.
ppdone)
-*/
dpu_encoder_prepare_for_kickoff(encoder, );
}
@@ -754,12 +746,8 @@ void dpu_crtc_commit_kickoff(struct drm_crtc
*crtc)
dpu_vbif_clear_errors(dpu_kms);
- list_for_each_entry(encoder, >mode_config.encoder_list, head)
{
- if (encoder->crtc != crtc)
- continue;
-
+ drm_for_each_encoder_mask(encoder, crtc->dev,
crtc->state->encoder_mask)
dpu_encoder_kickoff(encoder);
- }
We wont be holding the modeset locks here (and in crtc_atomic_begin) in
the display thread. Is
it safe to iterate over encoder_mask?
end:
reinit_completion(_crtc->frame_done_comp);
@@ -883,11 +871,8 @@ static void dpu_crtc_disable(struct drm_crtc
*crtc)
dpu_core_perf_crtc_update(crtc, 0, true);
- drm_for_each_encoder(encoder, crtc->dev) {
- if (encoder->crtc != crtc)
- continue;
+ 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;
@@ -922,12 +907,9 @@ static void dpu_crtc_enable(struct drm_crtc *crtc,
DRM_DEBUG_KMS("crtc%d\n", crtc->base.id);
dpu_crtc = to_dpu_crtc(crtc);
- drm_for_each_encoder(encoder, crtc->dev) {
- if (encoder->crtc != crtc)
- continue;
+ drm_for_each_encoder_mask(encoder, crtc->dev,
crtc->state->encoder_mask)
dpu_encoder_register_frame_event_callback(encoder,
dpu_crtc_frame_event_cb, (void *)crtc);
- }
mutex_lock(_crtc->crtc_lock);
trace_dpu_crtc_enable(DRMID(crtc), true, dpu_crtc);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 1bec4540f3e1..64134d619748 100644
---
Re: [Freedreno] [PATCH v2 14/24] drm/msm: dpu: Grab the modeset locks in frame_event
On 2018-11-16 10:42, Sean Paul wrote:
From: Sean Paul
This patch wraps dpu_core_perf_crtc_release_bw() with modeset locks
since it digs into the state objects.
Changes in v2:
- None
Signed-off-by: Sean Paul
---
Reviewed-by: Jeykumar Sankaran
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 80de5289ada3..156f4c77ca44 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -335,7 +335,9 @@ static void dpu_crtc_frame_event_work(struct
kthread_work *work)
/* release bandwidth and other resources */
trace_dpu_crtc_frame_event_done(DRMID(crtc),
fevent->event);
+ drm_modeset_lock_all(crtc->dev);
dpu_core_perf_crtc_release_bw(crtc);
+ drm_modeset_unlock_all(crtc->dev);
We might need to revisit this locking when we measure for performance as
it
could block the incoming frame locking.
} else {
trace_dpu_crtc_frame_event_more_pending(DRMID(crtc),
fevent->event);
--
Jeykumar S
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH v2 3/3] drm/msm: dpu: Make legacy cursor updates asynchronous
On Thu, Nov 08, 2018 at 02:00:51PM -0800, Jeykumar Sankaran wrote:
> On 2018-10-30 09:00, Sean Paul wrote:
> > From: Sean Paul
> >
> > This patch sprinkles a few async/legacy_cursor_update checks
> > through commit to ensure that cursor updates aren't blocked on vsync.
> > There are 2 main components to this, the first is that we don't want to
> > wait_for_commit_done in msm_atomic before returning from
> > atomic_complete.
> > The second is that in dpu we don't want to wait for frame_done events
> > when
> > updating the cursor.
> >
> > Changes in v2:
> > - None
> >
> > Signed-off-by: Sean Paul
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 44 +++--
> > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h| 3 +-
> > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 22 +++
> > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 6 ++-
> > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 5 ++-
> > drivers/gpu/drm/msm/msm_atomic.c| 3 +-
> > 6 files changed, 49 insertions(+), 34 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > index ed84cf44a222..1e3e57817b72 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > @@ -702,7 +702,7 @@ static int _dpu_crtc_wait_for_frame_done(struct
> > drm_crtc *crtc)
> > return rc;
> > }
> >
> > -void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
> > +void dpu_crtc_commit_kickoff(struct drm_crtc *crtc, bool async)
> > {
> > struct drm_encoder *encoder;
> > struct drm_device *dev = crtc->dev;
> > @@ -731,27 +731,30 @@ void dpu_crtc_commit_kickoff(struct drm_crtc
> > *crtc)
> > * Encoder will flush/start now, unless it has a tx
> > pending.
> > * If so, it may delay and flush at an irq event (e.g.
> > ppdone)
> > */
> > - dpu_encoder_prepare_for_kickoff(encoder, );
> > + dpu_encoder_prepare_for_kickoff(encoder, , async);
> > }
> >
> > - /* wait for frame_event_done completion */
> > - DPU_ATRACE_BEGIN("wait_for_frame_done_event");
> > - ret = _dpu_crtc_wait_for_frame_done(crtc);
> > - DPU_ATRACE_END("wait_for_frame_done_event");
> > - if (ret) {
> > - DPU_ERROR("crtc%d wait for frame done
> > failed;frame_pending%d\n",
> > - crtc->base.id,
> > - atomic_read(_crtc->frame_pending));
> > - goto end;
> > - }
> >
> > - if (atomic_inc_return(_crtc->frame_pending) == 1) {
> > - /* acquire bandwidth and other resources */
> > - DPU_DEBUG("crtc%d first commit\n", crtc->base.id);
> > - } else
> > - DPU_DEBUG("crtc%d commit\n", crtc->base.id);
> > + if (!async) {
> > + /* wait for frame_event_done completion */
> > + DPU_ATRACE_BEGIN("wait_for_frame_done_event");
> > + ret = _dpu_crtc_wait_for_frame_done(crtc);
> > + DPU_ATRACE_END("wait_for_frame_done_event");
> > + if (ret) {
> > + DPU_ERROR("crtc%d wait for frame done
> > failed;frame_pending%d\n",
> > + crtc->base.id,
> > +
> > atomic_read(_crtc->frame_pending));
> > + goto end;
> > + }
> > +
> > + if (atomic_inc_return(_crtc->frame_pending) == 1) {
> > + /* acquire bandwidth and other resources */
> > + DPU_DEBUG("crtc%d first commit\n", crtc->base.id);
> > + } else
> > + DPU_DEBUG("crtc%d commit\n", crtc->base.id);
> >
> > - dpu_crtc->play_count++;
> > + dpu_crtc->play_count++;
> > + }
> >
> > dpu_vbif_clear_errors(dpu_kms);
> >
> > @@ -759,11 +762,12 @@ void dpu_crtc_commit_kickoff(struct drm_crtc
> > *crtc)
> > if (encoder->crtc != crtc)
> > continue;
> >
> > - dpu_encoder_kickoff(encoder);
> > + dpu_encoder_kickoff(encoder, async);
> > }
> >
> > end:
> > - reinit_completion(_crtc->frame_done_comp);
> > + if (!async)
> > + reinit_completion(_crtc->frame_done_comp);
> > DPU_ATRACE_END("crtc_commit");
> > }
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> > index 4822602402f9..ec633ce3ee6c 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> > @@ -277,8 +277,9 @@ int dpu_crtc_vblank(struct drm_crtc *crtc, bool en);
> > /**
> > * dpu_crtc_commit_kickoff - trigger kickoff of the commit for this
> > crtc
> > * @crtc: Pointer to drm crtc object
> > + * @async: true if the commit is asynchronous, false otherwise
> > */
> > -void dpu_crtc_commit_kickoff(struct drm_crtc *crtc);
> > +void dpu_crtc_commit_kickoff(struct drm_crtc *crtc, bool async);
> >
> > /**
> > * dpu_crtc_complete_commit - callback signalling completion of current
> > commit
[Freedreno] [PATCH 2/2] drm/msm/dpu: add display port support in DPU
Add display port support in DPU by creating hooks
for DP encoder enumeration and encoder mode
initialization.
This change is based on the SDM845 Display port
driver changes[1].
[1] https://lwn.net/Articles/768265/
Signed-off-by: Jeykumar Sankaran
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 3 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 49 +
2 files changed, 46 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index b253165..e9c7edc6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2048,6 +2048,9 @@ static int dpu_encoder_setup_display(struct
dpu_encoder_virt *dpu_enc,
case DRM_MODE_ENCODER_DSI:
intf_type = INTF_DSI;
break;
+ case DRM_MODE_ENCODER_TMDS:
+ intf_type = INTF_DP;
+ break;
default:
DPU_ERROR_ENC(dpu_enc, "unsupported display interface type\n");
return -EINVAL;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 985c855..b823a37 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -473,6 +473,31 @@ static void _dpu_kms_initialize_dsi(struct drm_device *dev,
}
}
+static void _dpu_kms_initialize_displayport(struct drm_device *dev,
+ struct msm_drm_private *priv,
+ struct dpu_kms *dpu_kms)
+{
+ struct drm_encoder *encoder = NULL;
+ int rc;
+
+ if (!priv->dp)
+ return;
+
+ encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_TMDS);
+ if (IS_ERR_OR_NULL(encoder)) {
+ DPU_ERROR("encoder init failed for dsi display\n");
+ return;
+ }
+
+ priv->encoders[priv->num_encoders++] = encoder;
+
+ rc = msm_dp_modeset_init(priv->dp, dev, encoder);
+ if (rc) {
+ DPU_ERROR("modeset_init failed for DP, rc = %d\n", rc);
+ dpu_encoder_uninit(encoder);
+ }
+}
+
/**
* _dpu_kms_setup_displays - create encoders, bridges and connectors
* for underlying displays
@@ -487,6 +512,8 @@ static void _dpu_kms_setup_displays(struct drm_device *dev,
{
_dpu_kms_initialize_dsi(dev, priv, dpu_kms);
+ _dpu_kms_initialize_displayport(dev, priv, dpu_kms);
+
/**
* Extend this function to initialize other
* types of displays
@@ -723,13 +750,23 @@ static void _dpu_kms_set_encoder_mode(struct msm_kms *kms,
info.capabilities = cmd_mode ? MSM_DISPLAY_CAP_CMD_MODE :
MSM_DISPLAY_CAP_VID_MODE;
- /* TODO: No support for DSI swap */
- for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
- if (priv->dsi[i]) {
- info.h_tile_instance[info.num_of_h_tiles] = i;
- info.num_of_h_tiles++;
+ switch (info.intf_type) {
+ case DRM_MODE_ENCODER_DSI:
+ /* TODO: No support for DSI swap */
+ for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
+ if (priv->dsi[i]) {
+ info.h_tile_instance[info.num_of_h_tiles] = i;
+ info.num_of_h_tiles++;
+ }
}
- }
+ break;
+ case DRM_MODE_ENCODER_TMDS:
+ info.num_of_h_tiles = 1;
+ break;
+ default:
+ DPU_ERROR("Invalid connector type\n");
+ return;
+ };
rc = dpu_encoder_setup(encoder->dev, encoder, );
if (rc)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH 1/2] drm/msm/dpu: add dpu encoder uninit
Add encoder interface to release dpu encoder
on mode_init failures in kms.
Signed-off-by: Jeykumar Sankaran
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 12 ++--
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 6 ++
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index dd7ab85..b253165 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -422,6 +422,15 @@ void dpu_encoder_get_hw_resources(struct drm_encoder
*drm_enc,
}
}
+void dpu_encoder_uninit(struct drm_encoder *drm_enc)
+{
+ struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
+
+ drm_encoder_cleanup(drm_enc);
+
+ kfree(dpu_enc);
+}
+
static void dpu_encoder_destroy(struct drm_encoder *drm_enc)
{
struct dpu_encoder_virt *dpu_enc = NULL;
@@ -453,10 +462,9 @@ static void dpu_encoder_destroy(struct drm_encoder
*drm_enc)
dpu_enc->num_phys_encs = 0;
mutex_unlock(_enc->enc_lock);
- drm_encoder_cleanup(drm_enc);
mutex_destroy(_enc->enc_lock);
- kfree(dpu_enc);
+ dpu_encoder_uninit(drm_enc);
}
void dpu_encoder_helper_split_config(
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
index 9dbf38f..60b88bd 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
@@ -142,6 +142,12 @@ struct drm_encoder *dpu_encoder_init(
int drm_enc_mode);
/**
+ * dpu_encoder_uninit - uninitialize virtual encoder object
+ * @drm_enc: Pointer to drm encoder
+ */
+void dpu_encoder_uninit(struct drm_encoder *drm_enc);
+
+/**
* dpu_encoder_setup - setup dpu_encoder for the display probed
* @dev: Pointer to drm device structure
* @enc: Pointer to the drm_encoder
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v2 24/24] drm/msm: dpu: Remove crtc_lock
From: Sean Paul
Each time it's called we're holding the crtc modeset lock, so it's
redundant.
Changes in v2:
- None
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 11 ---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 3 ---
2 files changed, 14 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index f15cba2584a0..70b5104d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -69,7 +69,6 @@ static void dpu_crtc_destroy(struct drm_crtc *crtc)
return;
drm_crtc_cleanup(crtc);
- mutex_destroy(_crtc->crtc_lock);
kfree(dpu_crtc);
}
@@ -806,8 +805,6 @@ static void dpu_crtc_disable(struct drm_crtc *crtc,
old_crtc_state->encoder_mask)
dpu_encoder_assign_crtc(encoder, NULL);
- mutex_lock(_crtc->crtc_lock);
-
/* wait for frame_event_done completion */
if (_dpu_crtc_wait_for_frame_done(crtc))
DPU_ERROR("crtc%d wait for frame done failed;frame_pending%d\n",
@@ -836,8 +833,6 @@ static void dpu_crtc_disable(struct drm_crtc *crtc,
cstate->bw_control = false;
cstate->bw_split_vote = false;
- mutex_unlock(_crtc->crtc_lock);
-
if (crtc->state->event && !crtc->state->active) {
spin_lock_irqsave(>dev->event_lock, flags);
drm_crtc_send_vblank_event(crtc, crtc->state->event);
@@ -870,12 +865,9 @@ static void dpu_crtc_enable(struct drm_crtc *crtc,
dpu_encoder_register_frame_event_callback(encoder,
dpu_crtc_frame_event_cb, (void *)crtc);
- mutex_lock(_crtc->crtc_lock);
trace_dpu_crtc_enable(DRMID(crtc), true, dpu_crtc);
dpu_crtc->enabled = true;
- mutex_unlock(_crtc->crtc_lock);
-
drm_for_each_encoder_mask(encoder, crtc->dev, crtc->state->encoder_mask)
dpu_encoder_assign_crtc(encoder, crtc);
@@ -1177,7 +1169,6 @@ static int _dpu_debugfs_status_show(struct seq_file *s,
void *data)
drm_modeset_lock_all(crtc->dev);
cstate = to_dpu_crtc_state(crtc->state);
- mutex_lock(_crtc->crtc_lock);
mode = >state->adjusted_mode;
out_width = _dpu_crtc_get_mixer_width(cstate, mode);
@@ -1264,7 +1255,6 @@ static int _dpu_debugfs_status_show(struct seq_file *s,
void *data)
dpu_crtc->vblank_cb_time = ktime_set(0, 0);
}
- mutex_unlock(_crtc->crtc_lock);
drm_modeset_unlock_all(crtc->dev);
return 0;
@@ -1414,7 +1404,6 @@ struct drm_crtc *dpu_crtc_init(struct drm_device *dev,
struct drm_plane *plane,
crtc = _crtc->base;
crtc->dev = dev;
- mutex_init(_crtc->crtc_lock);
spin_lock_init(_crtc->spin_lock);
atomic_set(_crtc->frame_pending, 0);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index 2b358546af49..34f0c4d4d774 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -140,7 +140,6 @@ struct dpu_crtc_frame_event {
* @dirty_list: list of color processing features are dirty
* @ad_dirty: list containing ad properties that are dirty
* @ad_active: list containing ad properties that are active
- * @crtc_lock : crtc lock around create, destroy and access.
* @frame_pending : Whether or not an update is pending
* @frame_events : static allocation of in-flight frame events
* @frame_event_list : available frame event list
@@ -173,8 +172,6 @@ struct dpu_crtc {
struct list_head ad_dirty;
struct list_head ad_active;
- struct mutex crtc_lock;
-
atomic_t frame_pending;
struct dpu_crtc_frame_event frame_events[DPU_CRTC_FRAME_EVENT_SIZE];
struct list_head frame_event_list;
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v2 19/24] drm/msm: dpu: Remove vblank_callback from encoder
From: Sean Paul
The indirection of registering a callback and opaque pointer isn't reall
useful when there's only one callsite. So instead of having the
vblank_cb registration, just give encoder a crtc and let it directly
call the vblank handler.
In a later patch, we'll make use of this further.
Changes in v2:
- None
Cc: Jeykumar Sankaran
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 8 +++
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h| 6 +
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 25 +++--
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 10 -
4 files changed, 26 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 141ed1b0e90a..4d7f9ff1e9f4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -293,9 +293,8 @@ enum dpu_intf_mode dpu_crtc_get_intf_mode(struct drm_crtc
*crtc)
return INTF_MODE_NONE;
}
-static void dpu_crtc_vblank_cb(void *data)
+void dpu_crtc_vblank_callback(struct drm_crtc *crtc)
{
- struct drm_crtc *crtc = (struct drm_crtc *)data;
struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
/* keep statistics on vblank callback - with auto reset via debugfs */
@@ -771,8 +770,7 @@ static void _dpu_crtc_vblank_enable_no_lock(
DRMID(enc), enable,
dpu_crtc);
- dpu_encoder_register_vblank_callback(enc,
- dpu_crtc_vblank_cb, (void *)crtc);
+ dpu_encoder_assign_crtc(enc, crtc);
}
} else {
list_for_each_entry(enc, >mode_config.encoder_list, head) {
@@ -783,7 +781,7 @@ static void _dpu_crtc_vblank_enable_no_lock(
DRMID(enc), enable,
dpu_crtc);
- dpu_encoder_register_vblank_callback(enc, NULL, NULL);
+ dpu_encoder_assign_crtc(enc, NULL);
}
}
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index 93d21a61a040..54595cc29be5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -270,6 +270,12 @@ static inline int dpu_crtc_frame_pending(struct drm_crtc
*crtc)
*/
int dpu_crtc_vblank(struct drm_crtc *crtc, bool en);
+/**
+ * dpu_crtc_vblank_callback - called on vblank irq, issues completion events
+ * @crtc: Pointer to drm crtc object
+ */
+void dpu_crtc_vblank_callback(struct drm_crtc *crtc);
+
/**
* dpu_crtc_commit_kickoff - trigger kickoff of the commit for this crtc
* @crtc: Pointer to drm crtc object
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index d89ac520f7e6..fd6514f681ae 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -142,9 +142,11 @@ enum dpu_enc_rc_states {
* @intfs_swapped Whether or not the phys_enc interfaces have been swapped
* for partial update right-only cases, such as pingpong
* split where virtual pingpong does not generate IRQs
- * @crtc_vblank_cb:Callback into the upper layer / CRTC for
- * notification of the VBLANK
- * @crtc_vblank_cb_data: Data from upper layer for VBLANK notification
+ * @crtc: Pointer to the currently assigned crtc. Normally you
+ * would use crtc->state->encoder_mask to determine the
+ * link between encoder/crtc. However in this case we need
+ * to track crtc in the disable() hook which is called
+ * _after_ encoder_mask is cleared.
* @crtc_kickoff_cb: Callback into CRTC that will flush & start
* all CTL paths
* @crtc_kickoff_cb_data: Opaque user data given to crtc_kickoff_cb
@@ -186,8 +188,7 @@ struct dpu_encoder_virt {
bool intfs_swapped;
- void (*crtc_vblank_cb)(void *);
- void *crtc_vblank_cb_data;
+ struct drm_crtc *crtc;
struct dentry *debugfs_root;
struct mutex enc_lock;
@@ -1241,8 +1242,8 @@ static void dpu_encoder_vblank_callback(struct
drm_encoder *drm_enc,
dpu_enc = to_dpu_encoder_virt(drm_enc);
spin_lock_irqsave(_enc->enc_spinlock, lock_flags);
- if (dpu_enc->crtc_vblank_cb)
- dpu_enc->crtc_vblank_cb(dpu_enc->crtc_vblank_cb_data);
+ if (dpu_enc->crtc)
+ dpu_crtc_vblank_callback(dpu_enc->crtc);
spin_unlock_irqrestore(_enc->enc_spinlock, lock_flags);
atomic_inc(_enc->vsync_cnt);
@@ -1262,15 +1263,14 @@ static void dpu_encoder_underrun_callback(struct
[Freedreno] [PATCH v2 22/24] drm/msm: dpu: Separate crtc assignment from vblank enable
From: Sean Paul
Instead of assigning/clearing the crtc on vblank enable/disable, we can
just assign and clear the crtc on modeset. That allows us to just toggle
the encoder's vblank interrupts on vblank_enable.
So why is this important? Previously the driver was using the legacy
pointers to assign/clear the crtc. Legacy pointers are cleared _after_
disabling the hardware, so the legacy pointer was valid during
vblank_disable, but that's not something we should rely on.
Instead of relying on the core ordering the legacy pointer assignments
just so, we'll assign the crtc in dpu_crtc enable/disable. This is the
only place that mapping can change, so we're covered there.
We're also taking advantage of drm_crtc_vblank_on/off. By using this, we
ensure that vblank_enable/disable can never be called while the crtc is
off (which means the assigned crtc will always be valid). As such, we
don't need to use modeset locks or the crtc_lock in the
vblank_enable/disable routine to be sure state is consistent.
...I think.
Changes in v2:
- Changed crtc check in toggle_vblank to != (Jeykumar)
Cc: Jeykumar Sankaran
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 77 +
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 27 +---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 10 +++
3 files changed, 59 insertions(+), 55 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 54bb777b2d0c..725e15178cdb 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -749,43 +749,6 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
DPU_ATRACE_END("crtc_commit");
}
-/**
- * _dpu_crtc_vblank_enable_no_lock - update power resource and vblank request
- * @dpu_crtc: Pointer to dpu crtc structure
- * @enable: Whether to enable/disable vblanks
- */
-static void _dpu_crtc_vblank_enable_no_lock(
- struct dpu_crtc *dpu_crtc, bool enable)
-{
- struct drm_crtc *crtc = _crtc->base;
- struct drm_device *dev = crtc->dev;
- struct drm_encoder *enc;
-
- if (enable) {
- list_for_each_entry(enc, >mode_config.encoder_list, head) {
- if (enc->crtc != crtc)
- continue;
-
- trace_dpu_crtc_vblank_enable(DRMID(_crtc->base),
-DRMID(enc), enable,
-dpu_crtc);
-
- dpu_encoder_assign_crtc(enc, crtc);
- }
- } else {
- list_for_each_entry(enc, >mode_config.encoder_list, head) {
- if (enc->crtc != crtc)
- continue;
-
- trace_dpu_crtc_vblank_enable(DRMID(_crtc->base),
-DRMID(enc), enable,
-dpu_crtc);
-
- dpu_encoder_assign_crtc(enc, NULL);
- }
- }
-}
-
/**
* dpu_crtc_duplicate_state - state duplicate hook
* @crtc: Pointer to drm crtc structure
@@ -839,6 +802,10 @@ static void dpu_crtc_disable(struct drm_crtc *crtc,
/* Disable/save vblank irq handling */
drm_crtc_vblank_off(crtc);
+ drm_for_each_encoder_mask(encoder, crtc->dev,
+ old_crtc_state->encoder_mask)
+ dpu_encoder_assign_crtc(encoder, NULL);
+
mutex_lock(_crtc->crtc_lock);
/* wait for frame_event_done completion */
@@ -848,9 +815,6 @@ static void dpu_crtc_disable(struct drm_crtc *crtc,
atomic_read(_crtc->frame_pending));
trace_dpu_crtc_disable(DRMID(crtc), false, dpu_crtc);
- if (dpu_crtc->enabled && dpu_crtc->vblank_requested) {
- _dpu_crtc_vblank_enable_no_lock(dpu_crtc, false);
- }
dpu_crtc->enabled = false;
if (atomic_read(_crtc->frame_pending)) {
@@ -908,13 +872,13 @@ static void dpu_crtc_enable(struct drm_crtc *crtc,
mutex_lock(_crtc->crtc_lock);
trace_dpu_crtc_enable(DRMID(crtc), true, dpu_crtc);
- if (!dpu_crtc->enabled && dpu_crtc->vblank_requested) {
- _dpu_crtc_vblank_enable_no_lock(dpu_crtc, true);
- }
dpu_crtc->enabled = true;
mutex_unlock(_crtc->crtc_lock);
+ drm_for_each_encoder_mask(encoder, crtc->dev, crtc->state->encoder_mask)
+ dpu_encoder_assign_crtc(encoder, crtc);
+
/* Enable/restore vblank irq handling */
drm_crtc_vblank_on(crtc);
}
@@ -1159,10 +1123,33 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
int dpu_crtc_vblank(struct drm_crtc *crtc, bool en)
{
struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
+ struct drm_encoder *enc;
- mutex_lock(_crtc->crtc_lock);
trace_dpu_crtc_vblank(DRMID(_crtc->base),
[Freedreno] [PATCH v2 23/24] drm/msm: dpu: Remove vblank_requested flag from dpu_crtc
From: Sean Paul
It's just for debugfs output, we don't need it
Changes in v2:
- None
Cc: Jeykumar Sankaran
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 6 --
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 2 --
drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 14 --
3 files changed, 4 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 725e15178cdb..f15cba2584a0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1149,10 +1149,6 @@ int dpu_crtc_vblank(struct drm_crtc *crtc, bool en)
dpu_encoder_toggle_vblank_for_crtc(enc, crtc, en);
}
- mutex_lock(_crtc->crtc_lock);
- dpu_crtc->vblank_requested = en;
- mutex_unlock(_crtc->crtc_lock);
-
return 0;
}
@@ -1268,8 +1264,6 @@ static int _dpu_debugfs_status_show(struct seq_file *s,
void *data)
dpu_crtc->vblank_cb_time = ktime_set(0, 0);
}
- seq_printf(s, "vblank_enable:%d\n", dpu_crtc->vblank_requested);
-
mutex_unlock(_crtc->crtc_lock);
drm_modeset_unlock_all(crtc->dev);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index 54595cc29be5..2b358546af49 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -132,7 +132,6 @@ struct dpu_crtc_frame_event {
* @vblank_cb_count : count of vblank callback since last reset
* @play_count: frame count between crtc enable and disable
* @vblank_cb_time : ktime at vblank count reset
- * @vblank_requested : whether the user has requested vblank events
* @enabled : whether the DPU CRTC is currently enabled. updated in the
* commit-thread, not state-swap time which is earlier, so
* safe to make decisions on during VBLANK on/off work
@@ -166,7 +165,6 @@ struct dpu_crtc {
u32 vblank_cb_count;
u64 play_count;
ktime_t vblank_cb_time;
- bool vblank_requested;
bool enabled;
struct list_head feature_list;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
index 328df37d7580..c78b521ceda1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
@@ -728,20 +728,17 @@ TRACE_EVENT(dpu_crtc_vblank_enable,
__field(uint32_t, enc_id )
__field(bool, enable )
__field(bool, enabled )
- __field(bool, vblank_requested )
),
TP_fast_assign(
__entry->drm_id = drm_id;
__entry->enc_id = enc_id;
__entry->enable = enable;
__entry->enabled = crtc->enabled;
- __entry->vblank_requested = crtc->vblank_requested;
),
- TP_printk("id:%u encoder:%u enable:%s state{enabled:%s vblank_req:%s}",
+ TP_printk("id:%u encoder:%u enable:%s state{enabled:%s}",
__entry->drm_id, __entry->enc_id,
__entry->enable ? "true" : "false",
- __entry->enabled ? "true" : "false",
- __entry->vblank_requested ? "true" : "false")
+ __entry->enabled ? "true" : "false")
);
DECLARE_EVENT_CLASS(dpu_crtc_enable_template,
@@ -751,18 +748,15 @@ DECLARE_EVENT_CLASS(dpu_crtc_enable_template,
__field(uint32_t, drm_id )
__field(bool, enable )
__field(bool, enabled )
- __field(bool, vblank_requested )
),
TP_fast_assign(
__entry->drm_id = drm_id;
__entry->enable = enable;
__entry->enabled = crtc->enabled;
- __entry->vblank_requested = crtc->vblank_requested;
),
- TP_printk("id:%u enable:%s state{enabled:%s vblank_req:%s}",
+ TP_printk("id:%u enable:%s state{enabled:%s}",
__entry->drm_id, __entry->enable ? "true" : "false",
- __entry->enabled ? "true" : "false",
- __entry->vblank_requested ? "true" : "false")
+ __entry->enabled ? "true" : "false")
);
DEFINE_EVENT(dpu_crtc_enable_template, dpu_crtc_enable,
TP_PROTO(uint32_t drm_id, bool enable, struct dpu_crtc *crtc),
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v2 20/24] drm/msm: dpu: Use atomic_disable for dpu_crtc_disable
From: Sean Paul
Matches dpu_crtc_enable and we'll need the old state in a future patch
Changes in v2:
- None
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 4d7f9ff1e9f4..9efb41c7973b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -815,7 +815,8 @@ static struct drm_crtc_state
*dpu_crtc_duplicate_state(struct drm_crtc *crtc)
return >base;
}
-static void dpu_crtc_disable(struct drm_crtc *crtc)
+static void dpu_crtc_disable(struct drm_crtc *crtc,
+struct drm_crtc_state *old_crtc_state)
{
struct dpu_crtc *dpu_crtc;
struct dpu_crtc_state *cstate;
@@ -1407,7 +1408,7 @@ static const struct drm_crtc_funcs dpu_crtc_funcs = {
};
static const struct drm_crtc_helper_funcs dpu_crtc_helper_funcs = {
- .disable = dpu_crtc_disable,
+ .atomic_disable = dpu_crtc_disable,
.atomic_enable = dpu_crtc_enable,
.atomic_check = dpu_crtc_atomic_check,
.atomic_begin = dpu_crtc_atomic_begin,
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v2 21/24] drm/msm: dpu: Don't bother checking ->enabled in dpu_crtc_vblank
From: Sean Paul
The drm_crtc_vblank_on/off calls in enable/disable guarantee that we
won't call this function when crtc is not enabled.
Changes in v2:
- None
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 9efb41c7973b..54bb777b2d0c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1162,9 +1162,7 @@ int dpu_crtc_vblank(struct drm_crtc *crtc, bool en)
mutex_lock(_crtc->crtc_lock);
trace_dpu_crtc_vblank(DRMID(_crtc->base), en, dpu_crtc);
- if (dpu_crtc->enabled) {
- _dpu_crtc_vblank_enable_no_lock(dpu_crtc, en);
- }
+ _dpu_crtc_vblank_enable_no_lock(dpu_crtc, en);
dpu_crtc->vblank_requested = en;
mutex_unlock(_crtc->crtc_lock);
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v2 18/24] drm/msm: dpu: Remove crtc_lock from setup_mixers
From: Sean Paul
I think the intention here was to protect the enc->crtc access, but
that's insufficient to avoid enc->crtc changing. Fortunately we're
already holding the modeset lock when this is called (from
atomic_check), so remove the crtc_lock and add a modeset lock check.
While we're at it, use the encoder mask from crtc state instead of
legacy pointer.
Changes in v2:
- None
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index c49aaf412b6e..141ed1b0e90a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -474,19 +474,13 @@ static void _dpu_crtc_setup_mixer_for_encoder(
static void _dpu_crtc_setup_mixers(struct drm_crtc *crtc)
{
- struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
struct drm_encoder *enc;
- mutex_lock(_crtc->crtc_lock);
- /* Check for mixers on all encoders attached to this crtc */
- list_for_each_entry(enc, >dev->mode_config.encoder_list, head) {
- if (enc->crtc != crtc)
- continue;
+ WARN_ON(!drm_modeset_is_locked(>mutex));
+ /* Check for mixers on all encoders attached to this crtc */
+ drm_for_each_encoder_mask(enc, crtc->dev, crtc->state->encoder_mask)
_dpu_crtc_setup_mixer_for_encoder(crtc, enc);
- }
-
- mutex_unlock(_crtc->crtc_lock);
}
static void _dpu_crtc_setup_lm_bounds(struct drm_crtc *crtc,
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v2 16/24] drm/msm: dpu: Add modeset lock checks where applicable
From: Sean Paul
Add modeset lock checks to functions that could be called outside the
core atomic stack.
Changes in v2:
- None
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 +
2 files changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index a008a87a8113..cd0a0bea4335 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -284,6 +284,8 @@ enum dpu_intf_mode dpu_crtc_get_intf_mode(struct drm_crtc
*crtc)
return INTF_MODE_NONE;
}
+ WARN_ON(!drm_modeset_is_locked(>mutex));
+
/* TODO: Returns the first INTF_MODE, could there be multiple values? */
drm_for_each_encoder_mask(encoder, crtc->dev, crtc->state->encoder_mask)
return dpu_encoder_get_intf_mode(encoder);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 64134d619748..5104fc01147e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -358,6 +358,7 @@ void dpu_kms_encoder_enable(struct drm_encoder *encoder)
if (funcs && funcs->commit)
funcs->commit(encoder);
+ WARN_ON(!drm_modeset_is_locked(>mode_config.connection_mutex));
drm_for_each_crtc(crtc, dev) {
if (!(crtc->state->encoder_mask & drm_encoder_mask(encoder)))
continue;
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v2 11/24] drm/msm: dpu: Add ->enabled to dpu_encoder_virt
From: Sean Paul
Add a bool to dpu_encoder_virt to track whether the encoder is enabled
or not. Repurpose the enc_lock mutex to ensure that it is consistent
with the hw state.
Changes in v2:
- None
Cc: Jeykumar Sankaran
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 27 +
1 file changed, 22 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 10a0676d1dcf..3daa86220d47 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -132,6 +132,7 @@ enum dpu_enc_rc_states {
* @base: drm_encoder base class for registration with DRM
* @enc_spinlock: Virtual-Encoder-Wide Spin Lock for IRQ purposes
* @bus_scaling_client:Client handle to the bus scaling interface
+ * @enabled: True if the encoder is active, protected by enc_lock
* @num_phys_encs: Actual number of physical encoders contained.
* @phys_encs: Container of physical encoders managed.
* @cur_master:Pointer to the current master in this mode.
Optimization
@@ -148,8 +149,8 @@ enum dpu_enc_rc_states {
* all CTL paths
* @crtc_kickoff_cb_data: Opaque user data given to crtc_kickoff_cb
* @debugfs_root: Debug file system root file node
- * @enc_lock: Lock around physical encoder create/destroy and
- access.
+ * @enc_lock: Lock around physical encoder
+ * create/destroy/enable/disable
* @frame_busy_mask: Bitmask tracking which phys_enc we are still
* busy processing current command.
* Bit0 = phys_encs[0] etc.
@@ -175,6 +176,8 @@ struct dpu_encoder_virt {
spinlock_t enc_spinlock;
uint32_t bus_scaling_client;
+ bool enabled;
+
unsigned int num_phys_encs;
struct dpu_encoder_phys *phys_encs[MAX_PHYS_ENCODERS_PER_VIRTUAL];
struct dpu_encoder_phys *cur_master;
@@ -1121,6 +1124,8 @@ static void dpu_encoder_virt_enable(struct drm_encoder
*drm_enc)
return;
}
dpu_enc = to_dpu_encoder_virt(drm_enc);
+
+ mutex_lock(_enc->enc_lock);
cur_mode = _enc->base.crtc->state->adjusted_mode;
trace_dpu_enc_enable(DRMID(drm_enc), cur_mode->hdisplay,
@@ -1137,10 +1142,15 @@ static void dpu_encoder_virt_enable(struct drm_encoder
*drm_enc)
if (ret) {
DPU_ERROR_ENC(dpu_enc, "dpu resource control failed: %d\n",
ret);
- return;
+ goto out;
}
_dpu_encoder_virt_enable_helper(drm_enc);
+
+ dpu_enc->enabled = true;
+
+out:
+ mutex_unlock(_enc->enc_lock);
}
static void dpu_encoder_virt_disable(struct drm_encoder *drm_enc)
@@ -1162,11 +1172,14 @@ static void dpu_encoder_virt_disable(struct drm_encoder
*drm_enc)
return;
}
- mode = _enc->crtc->state->adjusted_mode;
-
dpu_enc = to_dpu_encoder_virt(drm_enc);
DPU_DEBUG_ENC(dpu_enc, "\n");
+ mutex_lock(_enc->enc_lock);
+ dpu_enc->enabled = false;
+
+ mode = _enc->crtc->state->adjusted_mode;
+
priv = drm_enc->dev->dev_private;
dpu_kms = to_dpu_kms(priv->kms);
@@ -1200,6 +1213,8 @@ static void dpu_encoder_virt_disable(struct drm_encoder
*drm_enc)
DPU_DEBUG_ENC(dpu_enc, "encoder disabled\n");
dpu_rm_release(_kms->rm, drm_enc);
+
+ mutex_unlock(_enc->enc_lock);
}
static enum dpu_intf dpu_encoder_get_intf(struct dpu_mdss_cfg *catalog,
@@ -2233,6 +2248,8 @@ struct drm_encoder *dpu_encoder_init(struct drm_device
*dev,
drm_encoder_helper_add(_enc->base, _encoder_helper_funcs);
+ dpu_enc->enabled = false;
+
return _enc->base;
}
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v2 12/24] drm/msm: dpu: Move crtc runtime resume to encoder
From: Sean Paul
The crtc runtime resume doesn't actually operate on the crtc, but rather
its encoders. The problem with this is that we need to inspect the crtc
state to get the currently connected encoders. Since runtime resume
isn't guaranteed to be called while holding the modeset locks (although
it sometimes is), this presents a race condition.
Now that we have ->enabled on the virtual encoders, and a lock to
protect it, just call resume on each encoder and only restore the ones
that are enabled.
Changes in v2:
- None
Cc: Jeykumar Sankaran
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 24 -
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h| 6 --
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 24 +
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 4 ++--
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 6 +++---
5 files changed, 15 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index d8f58caf2772..9be24907f8c1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -841,30 +841,6 @@ static struct drm_crtc_state
*dpu_crtc_duplicate_state(struct drm_crtc *crtc)
return >base;
}
-void dpu_crtc_runtime_resume(struct drm_crtc *crtc)
-{
- struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
- struct drm_encoder *encoder;
-
- mutex_lock(_crtc->crtc_lock);
-
- if (!dpu_crtc->enabled)
- goto end;
-
- trace_dpu_crtc_runtime_resume(DRMID(crtc));
-
- /* restore encoder; crtc will be programmed during commit */
- drm_for_each_encoder(encoder, crtc->dev) {
- if (encoder->crtc != crtc)
- continue;
-
- dpu_encoder_virt_restore(encoder);
- }
-
-end:
- mutex_unlock(_crtc->crtc_lock);
-}
-
static void dpu_crtc_disable(struct drm_crtc *crtc)
{
struct dpu_crtc *dpu_crtc;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index 1dca91d1210f..93d21a61a040 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -329,10 +329,4 @@ static inline bool dpu_crtc_is_enabled(struct drm_crtc
*crtc)
return crtc ? crtc->enabled : false;
}
-/**
- * dpu_crtc_runtime_resume - called by the top-level on pm_runtime_resume
- * @crtc: CRTC to resume
- */
-void dpu_crtc_runtime_resume(struct drm_crtc *crtc);
-
#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 3daa86220d47..d89ac520f7e6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1089,28 +1089,24 @@ static void _dpu_encoder_virt_enable_helper(struct
drm_encoder *drm_enc)
_dpu_encoder_update_vsync_source(dpu_enc, _enc->disp_info);
}
-void dpu_encoder_virt_restore(struct drm_encoder *drm_enc)
+void dpu_encoder_virt_runtime_resume(struct drm_encoder *drm_enc)
{
- struct dpu_encoder_virt *dpu_enc = NULL;
- int i;
-
- if (!drm_enc) {
- DPU_ERROR("invalid encoder\n");
- return;
- }
- dpu_enc = to_dpu_encoder_virt(drm_enc);
+ struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
- for (i = 0; i < dpu_enc->num_phys_encs; i++) {
- struct dpu_encoder_phys *phys = dpu_enc->phys_encs[i];
+ mutex_lock(_enc->enc_lock);
- if (phys && (phys != dpu_enc->cur_master) && phys->ops.restore)
- phys->ops.restore(phys);
- }
+ if (!dpu_enc->enabled)
+ goto out;
+ if (dpu_enc->cur_slave && dpu_enc->cur_slave->ops.restore)
+ dpu_enc->cur_slave->ops.restore(dpu_enc->cur_slave);
if (dpu_enc->cur_master && dpu_enc->cur_master->ops.restore)
dpu_enc->cur_master->ops.restore(dpu_enc->cur_master);
_dpu_encoder_virt_enable_helper(drm_enc);
+
+out:
+ mutex_unlock(_enc->enc_lock);
}
static void dpu_encoder_virt_enable(struct drm_encoder *drm_enc)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
index 9dbf38f446d9..aa4f135218fa 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
@@ -126,10 +126,10 @@ int dpu_encoder_wait_for_event(struct drm_encoder
*drm_encoder,
enum dpu_intf_mode dpu_encoder_get_intf_mode(struct drm_encoder *encoder);
/**
- * dpu_encoder_virt_restore - restore the encoder configs
+ * dpu_encoder_virt_runtime_resume - pm runtime resume the encoder configs
* @encoder: encoder pointer
*/
-void dpu_encoder_virt_restore(struct drm_encoder *encoder);
+void dpu_encoder_virt_runtime_resume(struct drm_encoder *encoder);
/**
* dpu_encoder_init - initialize virtual encoder
[Freedreno] [PATCH v2 17/24] drm/msm: dpu: Move pm_runtime_(get|put) from vblank_enable
From: Sean Paul
There are 4 times that _dpu_crtc_vblank_enable_no_lock() is called:
1- crtc enable
2- crtc disable
3- crtc vblank enable
4- crtc vblank disable
When we enable or disable the crtc, we call drm_crtc_vblank_on and
drm_crtc_vblank_off respectively. That will gate vblank enables and
disables to only being called when the crtc is active. That means that
we can just enable/disable pm runtime in crtc enable/disable. This will
be beneficial in trying to eliminate blocking calls from the vblank call
chain.
Changes in v2:
- None
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 8
1 file changed, 4 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 cd0a0bea4335..c49aaf412b6e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -769,8 +769,6 @@ static void _dpu_crtc_vblank_enable_no_lock(
struct drm_encoder *enc;
if (enable) {
- pm_runtime_get_sync(dev->dev);
-
list_for_each_entry(enc, >mode_config.encoder_list, head) {
if (enc->crtc != crtc)
continue;
@@ -793,8 +791,6 @@ static void _dpu_crtc_vblank_enable_no_lock(
dpu_encoder_register_vblank_callback(enc, NULL, NULL);
}
-
- pm_runtime_put_sync(dev->dev);
}
}
@@ -891,6 +887,8 @@ static void dpu_crtc_disable(struct drm_crtc *crtc)
crtc->state->event = NULL;
spin_unlock_irqrestore(>dev->event_lock, flags);
}
+
+ pm_runtime_put_sync(crtc->dev->dev);
}
static void dpu_crtc_enable(struct drm_crtc *crtc,
@@ -906,6 +904,8 @@ static void dpu_crtc_enable(struct drm_crtc *crtc,
}
priv = crtc->dev->dev_private;
+ pm_runtime_get_sync(crtc->dev->dev);
+
DRM_DEBUG_KMS("crtc%d\n", crtc->base.id);
dpu_crtc = to_dpu_crtc(crtc);
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v2 13/24] drm/msm: dpu: Don't drop locks in crtc_vblank_enable
From: Sean Paul
Now that runtime resume is handled in encoder, we don't need to worry
about crtc_lock recursion when calling pm_runtime_(get|put). So drop the
lock drops in _dpu_crtc_vblank_enable_no_lock().
Changes in v2:
- None
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 9be24907f8c1..80de5289ada3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -777,10 +777,7 @@ static void _dpu_crtc_vblank_enable_no_lock(
struct drm_encoder *enc;
if (enable) {
- /* drop lock since power crtc cb may try to re-acquire lock */
- mutex_unlock(_crtc->crtc_lock);
pm_runtime_get_sync(dev->dev);
- mutex_lock(_crtc->crtc_lock);
list_for_each_entry(enc, >mode_config.encoder_list, head) {
if (enc->crtc != crtc)
@@ -805,10 +802,7 @@ static void _dpu_crtc_vblank_enable_no_lock(
dpu_encoder_register_vblank_callback(enc, NULL, NULL);
}
- /* drop lock since power crtc cb may try to re-acquire lock */
- mutex_unlock(_crtc->crtc_lock);
pm_runtime_put_sync(dev->dev);
- mutex_lock(_crtc->crtc_lock);
}
}
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v2 14/24] drm/msm: dpu: Grab the modeset locks in frame_event
From: Sean Paul
This patch wraps dpu_core_perf_crtc_release_bw() with modeset locks
since it digs into the state objects.
Changes in v2:
- None
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 80de5289ada3..156f4c77ca44 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -335,7 +335,9 @@ static void dpu_crtc_frame_event_work(struct kthread_work
*work)
/* release bandwidth and other resources */
trace_dpu_crtc_frame_event_done(DRMID(crtc),
fevent->event);
+ drm_modeset_lock_all(crtc->dev);
dpu_core_perf_crtc_release_bw(crtc);
+ drm_modeset_unlock_all(crtc->dev);
} else {
trace_dpu_crtc_frame_event_more_pending(DRMID(crtc),
fevent->event);
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v2 15/24] drm/msm: dpu: Stop using encoder->crtc pointer
From: Sean Paul
It's for legacy drivers, for atomic drivers crtc->state->encoder_mask
should be used to map encoder to crtc.
Changes in v2:
- None
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 46
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 19 +++---
2 files changed, 29 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 156f4c77ca44..a008a87a8113 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -284,9 +284,9 @@ enum dpu_intf_mode dpu_crtc_get_intf_mode(struct drm_crtc
*crtc)
return INTF_MODE_NONE;
}
- drm_for_each_encoder(encoder, crtc->dev)
- if (encoder->crtc == crtc)
- return dpu_encoder_get_intf_mode(encoder);
+ /* TODO: Returns the first INTF_MODE, could there be multiple values? */
+ drm_for_each_encoder_mask(encoder, crtc->dev, crtc->state->encoder_mask)
+ return dpu_encoder_get_intf_mode(encoder);
return INTF_MODE_NONE;
}
@@ -551,13 +551,9 @@ static void dpu_crtc_atomic_begin(struct drm_crtc *crtc,
spin_unlock_irqrestore(>event_lock, flags);
}
- list_for_each_entry(encoder, >mode_config.encoder_list, head) {
- if (encoder->crtc != crtc)
- continue;
-
- /* encoder will trigger pending mask now */
+ /* encoder will trigger pending mask now */
+ drm_for_each_encoder_mask(encoder, crtc->dev, crtc->state->encoder_mask)
dpu_encoder_trigger_kickoff_pending(encoder);
- }
/*
* If no mixers have been allocated in dpu_crtc_atomic_check(),
@@ -704,7 +700,6 @@ static int _dpu_crtc_wait_for_frame_done(struct drm_crtc
*crtc)
void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
{
struct drm_encoder *encoder;
- struct drm_device *dev = crtc->dev;
struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc->state);
@@ -720,16 +715,13 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
DPU_ATRACE_BEGIN("crtc_commit");
- list_for_each_entry(encoder, >mode_config.encoder_list, head) {
+ /*
+* Encoder will flush/start now, unless it has a tx pending. If so, it
+* may delay and flush at an irq event (e.g. ppdone)
+*/
+ drm_for_each_encoder_mask(encoder, crtc->dev,
+ crtc->state->encoder_mask) {
struct dpu_encoder_kickoff_params params = { 0 };
-
- if (encoder->crtc != crtc)
- continue;
-
- /*
-* Encoder will flush/start now, unless it has a tx pending.
-* If so, it may delay and flush at an irq event (e.g. ppdone)
-*/
dpu_encoder_prepare_for_kickoff(encoder, );
}
@@ -754,12 +746,8 @@ void dpu_crtc_commit_kickoff(struct drm_crtc *crtc)
dpu_vbif_clear_errors(dpu_kms);
- list_for_each_entry(encoder, >mode_config.encoder_list, head) {
- if (encoder->crtc != crtc)
- continue;
-
+ drm_for_each_encoder_mask(encoder, crtc->dev, crtc->state->encoder_mask)
dpu_encoder_kickoff(encoder);
- }
end:
reinit_completion(_crtc->frame_done_comp);
@@ -883,11 +871,8 @@ static void dpu_crtc_disable(struct drm_crtc *crtc)
dpu_core_perf_crtc_update(crtc, 0, true);
- drm_for_each_encoder(encoder, crtc->dev) {
- if (encoder->crtc != crtc)
- continue;
+ 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;
@@ -922,12 +907,9 @@ static void dpu_crtc_enable(struct drm_crtc *crtc,
DRM_DEBUG_KMS("crtc%d\n", crtc->base.id);
dpu_crtc = to_dpu_crtc(crtc);
- drm_for_each_encoder(encoder, crtc->dev) {
- if (encoder->crtc != crtc)
- continue;
+ drm_for_each_encoder_mask(encoder, crtc->dev, crtc->state->encoder_mask)
dpu_encoder_register_frame_event_callback(encoder,
dpu_crtc_frame_event_cb, (void *)crtc);
- }
mutex_lock(_crtc->crtc_lock);
trace_dpu_crtc_enable(DRMID(crtc), true, dpu_crtc);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 1bec4540f3e1..64134d619748 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -320,7 +320,10 @@ static void dpu_kms_prepare_commit(struct msm_kms
[Freedreno] [PATCH v2 08/24] drm/msm: dpu: Move DPU_POWER_HANDLE_DBUS_ID to core_perf
From: Sean Paul
It's only used in core_perf, so stick it there (and change the name to
reflect that).
Changes in v2:
- None
Reviewed-by: Jeykumar Sankaran
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 34 +--
drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h | 17 --
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 4 +--
.../gpu/drm/msm/disp/dpu1/dpu_power_handle.h | 13 ---
4 files changed, 34 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
index ef6dd43f8bec..bffc51e496e7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
@@ -95,20 +95,20 @@ static void _dpu_core_perf_calc_crtc(struct dpu_kms *kms,
memset(perf, 0, sizeof(struct dpu_core_perf_params));
if (!dpu_cstate->bw_control) {
- for (i = 0; i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
+ for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
perf->bw_ctl[i] = kms->catalog->perf.max_bw_high *
1000ULL;
perf->max_per_pipe_ib[i] = perf->bw_ctl[i];
}
perf->core_clk_rate = kms->perf.max_core_clk_rate;
} else if (kms->perf.perf_tune.mode == DPU_PERF_MODE_MINIMUM) {
- for (i = 0; i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
+ for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
perf->bw_ctl[i] = 0;
perf->max_per_pipe_ib[i] = 0;
}
perf->core_clk_rate = 0;
} else if (kms->perf.perf_tune.mode == DPU_PERF_MODE_FIXED) {
- for (i = 0; i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
+ for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
perf->bw_ctl[i] = kms->perf.fix_core_ab_vote;
perf->max_per_pipe_ib[i] = kms->perf.fix_core_ib_vote;
}
@@ -118,12 +118,12 @@ static void _dpu_core_perf_calc_crtc(struct dpu_kms *kms,
DPU_DEBUG(
"crtc=%d clk_rate=%llu core_ib=%llu core_ab=%llu llcc_ib=%llu
llcc_ab=%llu mem_ib=%llu mem_ab=%llu\n",
crtc->base.id, perf->core_clk_rate,
- perf->max_per_pipe_ib[DPU_POWER_HANDLE_DBUS_ID_MNOC],
- perf->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_MNOC],
- perf->max_per_pipe_ib[DPU_POWER_HANDLE_DBUS_ID_LLCC],
- perf->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_LLCC],
- perf->max_per_pipe_ib[DPU_POWER_HANDLE_DBUS_ID_EBI],
- perf->bw_ctl[DPU_POWER_HANDLE_DBUS_ID_EBI]);
+ perf->max_per_pipe_ib[DPU_CORE_PERF_DATA_BUS_ID_MNOC],
+ perf->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_MNOC],
+ perf->max_per_pipe_ib[DPU_CORE_PERF_DATA_BUS_ID_LLCC],
+ perf->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_LLCC],
+ perf->max_per_pipe_ib[DPU_CORE_PERF_DATA_BUS_ID_EBI],
+ perf->bw_ctl[DPU_CORE_PERF_DATA_BUS_ID_EBI]);
}
int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
@@ -158,8 +158,8 @@ int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
/* obtain new values */
_dpu_core_perf_calc_crtc(kms, crtc, state, _cstate->new_perf);
- for (i = DPU_POWER_HANDLE_DBUS_ID_MNOC;
- i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
+ for (i = DPU_CORE_PERF_DATA_BUS_ID_MNOC;
+ i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
bw_sum_of_intfs = dpu_cstate->new_perf.bw_ctl[i];
curr_client_type = dpu_crtc_get_client_type(crtc);
@@ -290,7 +290,7 @@ void dpu_core_perf_crtc_release_bw(struct drm_crtc *crtc)
if (kms->perf.enable_bw_release) {
trace_dpu_cmd_release_bw(crtc->base.id);
DPU_DEBUG("Release BW crtc=%d\n", crtc->base.id);
- for (i = 0; i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
+ for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
dpu_crtc->cur_perf.bw_ctl[i] = 0;
_dpu_core_perf_crtc_update_bus(kms, crtc, i);
}
@@ -367,7 +367,7 @@ int dpu_core_perf_crtc_update(struct drm_crtc *crtc,
new = _cstate->new_perf;
if (_dpu_core_perf_crtc_is_power_on(crtc) && !stop_req) {
- for (i = 0; i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
+ for (i = 0; i < DPU_CORE_PERF_DATA_BUS_ID_MAX; i++) {
/*
* cases for bus bandwidth update.
* 1. new bandwidth vote - "ab or ib vote" is higher
@@ -409,13 +409,13 @@ int dpu_core_perf_crtc_update(struct drm_crtc *crtc,
update_clk = 1;
}
[Freedreno] [PATCH v2 10/24] drm/msm: dpu: Fix typo in dpu_encoder
From: Sean Paul
enc_spinlock instead of enc_spin_lock.
Changes in v2:
- None
Reviewed-by: Jeykumar Sankaran
Signed-off-by: Sean Paul
---
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 82c55efb500f..10a0676d1dcf 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -130,7 +130,7 @@ enum dpu_enc_rc_states {
* Virtual encoder defers as much as possible to the physical encoders.
* Virtual encoder registers itself with the DRM Framework as the encoder.
* @base: drm_encoder base class for registration with DRM
- * @enc_spin_lock: Virtual-Encoder-Wide Spin Lock for IRQ purposes
+ * @enc_spinlock: Virtual-Encoder-Wide Spin Lock for IRQ purposes
* @bus_scaling_client:Client handle to the bus scaling interface
* @num_phys_encs: Actual number of physical encoders contained.
* @phys_encs: Container of physical encoders managed.
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v2 09/24] drm/msm: dpu: Remove dpu_power_handle
From: Sean Paul
Now that we don't have any event handlers, remove dpu_power_handle!
Changes in v2:
- None
Reviewed-by: Jeykumar Sankaran
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/Makefile | 1 -
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 11 --
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 3 -
.../gpu/drm/msm/disp/dpu1/dpu_power_handle.c | 136 --
.../gpu/drm/msm/disp/dpu1/dpu_power_handle.h | 113 ---
5 files changed, 264 deletions(-)
delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index 19ab521d4c3a..7d02ef3655b5 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -72,7 +72,6 @@ msm-y := \
disp/dpu1/dpu_kms.o \
disp/dpu1/dpu_mdss.o \
disp/dpu1/dpu_plane.o \
- disp/dpu1/dpu_power_handle.o \
disp/dpu1/dpu_rm.o \
disp/dpu1/dpu_vbif.o \
msm_atomic.o \
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index af666d917a0b..9f7da56bb453 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1060,8 +1060,6 @@ static int dpu_bind(struct device *dev, struct device
*master, void *data)
return ret;
}
- dpu_power_resource_init(pdev, _kms->phandle);
-
platform_set_drvdata(pdev, dpu_kms);
msm_kms_init(_kms->base, _funcs);
@@ -1081,7 +1079,6 @@ static void dpu_unbind(struct device *dev, struct device
*master, void *data)
struct dpu_kms *dpu_kms = platform_get_drvdata(pdev);
struct dss_module_power *mp = _kms->mp;
- dpu_power_resource_deinit(pdev, _kms->phandle);
msm_dss_put_clk(mp->clk_config, mp->num_clk);
devm_kfree(>dev, mp->clk_config);
mp->num_clk = 0;
@@ -1120,10 +1117,6 @@ static int __maybe_unused dpu_runtime_suspend(struct
device *dev)
return rc;
}
- rc = dpu_power_resource_enable(_kms->phandle, false);
- if (rc)
- DPU_ERROR("resource disable failed: %d\n", rc);
-
rc = msm_dss_enable_clk(mp->clk_config, mp->num_clk, false);
if (rc)
DPU_ERROR("clock disable failed rc:%d\n", rc);
@@ -1157,10 +1150,6 @@ static int __maybe_unused dpu_runtime_resume(struct
device *dev)
drm_for_each_crtc(crtc, ddev)
dpu_crtc_runtime_resume(crtc);
- rc = dpu_power_resource_enable(_kms->phandle, true);
- if (rc)
- DPU_ERROR("resource enable failed: %d\n", rc);
-
return rc;
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index 4e5acacb3065..59e18e2d3c59 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -31,7 +31,6 @@
#include "dpu_hw_top.h"
#include "dpu_io_util.h"
#include "dpu_rm.h"
-#include "dpu_power_handle.h"
#include "dpu_irq.h"
#include "dpu_core_perf.h"
@@ -114,8 +113,6 @@ struct dpu_kms {
int core_rev;
struct dpu_mdss_cfg *catalog;
- struct dpu_power_handle phandle;
-
/* directory entry for debugfs */
struct dentry *debugfs_root;
struct dentry *debugfs_danger;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
deleted file mode 100644
index 8e64f0a52147..
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
+++ /dev/null
@@ -1,136 +0,0 @@
-/* Copyright (c) 2014-2018, The Linux Foundation. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 and
- * only version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- */
-
-#define pr_fmt(fmt)"[drm:%s:%d]: " fmt, __func__, __LINE__
-
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-
-#include "dpu_power_handle.h"
-#include "dpu_trace.h"
-
-static void dpu_power_event_trigger_locked(struct dpu_power_handle *phandle,
- u32 event_type)
-{
- struct dpu_power_event *event;
-
- list_for_each_entry(event, >event_list, list) {
- if (event->event_type & event_type)
- event->cb_fnc(event_type, event->usr);
- }
-}
-
-void dpu_power_resource_init(struct platform_device *pdev,
- struct dpu_power_handle *phandle)
-{
- phandle->dev = >dev;
-
- INIT_LIST_HEAD(>event_list);
-
- mutex_init(>phandle_lock);
-}
-
-void
[Freedreno] [PATCH v2 05/24] drm/msm: dpu: Handle crtc pm_runtime_resume() directly
From: Sean Paul
Instead of registering through dpu_power_handle just to get a call on
runtime_resume, call the crtc function directly.
Changes in v2:
- None
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 23 ++-
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 10 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 4
drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 8
4 files changed, 20 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index e09209d6c469..c55cb751e2b4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -33,7 +33,6 @@
#include "dpu_plane.h"
#include "dpu_encoder.h"
#include "dpu_vbif.h"
-#include "dpu_power_handle.h"
#include "dpu_core_perf.h"
#include "dpu_trace.h"
@@ -69,8 +68,6 @@ static void dpu_crtc_destroy(struct drm_crtc *crtc)
if (!crtc)
return;
- dpu_crtc->phandle = NULL;
-
drm_crtc_cleanup(crtc);
mutex_destroy(_crtc->crtc_lock);
kfree(dpu_crtc);
@@ -844,15 +841,17 @@ static struct drm_crtc_state
*dpu_crtc_duplicate_state(struct drm_crtc *crtc)
return >base;
}
-static void dpu_crtc_handle_power_event(u32 event_type, void *arg)
+void dpu_crtc_runtime_resume(struct drm_crtc *crtc)
{
- struct drm_crtc *crtc = arg;
struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
struct drm_encoder *encoder;
mutex_lock(_crtc->crtc_lock);
- trace_dpu_crtc_handle_power_event(DRMID(crtc), event_type);
+ if (!dpu_crtc->enabled)
+ goto end;
+
+ trace_dpu_crtc_runtime_resume(DRMID(crtc));
/* restore encoder; crtc will be programmed during commit */
drm_for_each_encoder(encoder, crtc->dev) {
@@ -862,6 +861,7 @@ static void dpu_crtc_handle_power_event(u32 event_type,
void *arg)
dpu_encoder_virt_restore(encoder);
}
+end:
mutex_unlock(_crtc->crtc_lock);
}
@@ -917,10 +917,6 @@ static void dpu_crtc_disable(struct drm_crtc *crtc)
dpu_encoder_register_frame_event_callback(encoder, NULL, NULL);
}
- if (dpu_crtc->power_event)
- dpu_power_handle_unregister_event(dpu_crtc->phandle,
- dpu_crtc->power_event);
-
memset(cstate->mixers, 0, sizeof(cstate->mixers));
cstate->num_mixers = 0;
@@ -972,11 +968,6 @@ static void dpu_crtc_enable(struct drm_crtc *crtc,
/* Enable/restore vblank irq handling */
drm_crtc_vblank_on(crtc);
-
- dpu_crtc->power_event = dpu_power_handle_register_event(
- dpu_crtc->phandle, DPU_POWER_EVENT_ENABLE,
- dpu_crtc_handle_power_event, crtc, dpu_crtc->name);
-
}
struct plane_state {
@@ -1522,8 +1513,6 @@ struct drm_crtc *dpu_crtc_init(struct drm_device *dev,
struct drm_plane *plane,
/* initialize event handling */
spin_lock_init(_crtc->event_lock);
- dpu_crtc->phandle = >phandle;
-
DPU_DEBUG("%s: successfully initialized crtc\n", dpu_crtc->name);
return crtc;
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
index 4822602402f9..1dca91d1210f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
@@ -151,7 +151,6 @@ struct dpu_crtc_frame_event {
* @event_worker : Event worker queue
* @event_lock: Spinlock around event handling code
* @phandle: Pointer to power handler
- * @power_event : registered power event handle
* @cur_perf : current performance committed to clock/bandwidth driver
*/
struct dpu_crtc {
@@ -187,9 +186,6 @@ struct dpu_crtc {
/* for handling internal event thread */
spinlock_t event_lock;
- struct dpu_power_handle *phandle;
- struct dpu_power_event *power_event;
-
struct dpu_core_perf_params cur_perf;
struct dpu_crtc_smmu_state_data smmu_state;
@@ -333,4 +329,10 @@ static inline bool dpu_crtc_is_enabled(struct drm_crtc
*crtc)
return crtc ? crtc->enabled : false;
}
+/**
+ * dpu_crtc_runtime_resume - called by the top-level on pm_runtime_resume
+ * @crtc: CRTC to resume
+ */
+void dpu_crtc_runtime_resume(struct drm_crtc *crtc);
+
#endif /* _DPU_CRTC_H_ */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index ab8574ab8327..654ea5060e02 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -1137,6 +1137,7 @@ static int __maybe_unused dpu_runtime_resume(struct
device *dev)
int rc = -1;
struct platform_device *pdev = to_platform_device(dev);
struct dpu_kms *dpu_kms = platform_get_drvdata(pdev);
+ struct drm_crtc *crtc;
struct drm_device *ddev;
struct dss_module_power *mp = _kms->mp;
@@ -1154,6 +1155,9
[Freedreno] [PATCH v2 02/24] drm/msm: dpu: Remove unused trace_dpu_perf_update_bus()
From: Sean Paul
Changes in v2:
- None
Reviewed-by: Jeykumar Sankaran
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 21 -
1 file changed, 21 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
index 0c122e173892..7ab0ba8224f6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h
@@ -99,27 +99,6 @@ TRACE_EVENT(dpu_perf_set_ot,
__entry->vbif_idx)
)
-TRACE_EVENT(dpu_perf_update_bus,
- TP_PROTO(int client, unsigned long long ab_quota,
- unsigned long long ib_quota),
- TP_ARGS(client, ab_quota, ib_quota),
- TP_STRUCT__entry(
- __field(int, client)
- __field(u64, ab_quota)
- __field(u64, ib_quota)
- ),
- TP_fast_assign(
- __entry->client = client;
- __entry->ab_quota = ab_quota;
- __entry->ib_quota = ib_quota;
- ),
- TP_printk("Request client:%d ab=%llu ib=%llu",
- __entry->client,
- __entry->ab_quota,
- __entry->ib_quota)
-)
-
-
TRACE_EVENT(dpu_cmd_release_bw,
TP_PROTO(u32 crtc_id),
TP_ARGS(crtc_id),
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v2 03/24] drm/msm: dpu: Remove dpu_power_client
From: Sean Paul
There's only one client -- core, and it's only used for runtime pm which
is already refcounted.
Changes in v2:
- None
Reviewed-by: Jeykumar Sankaran
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 22 +
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 1 -
.../gpu/drm/msm/disp/dpu1/dpu_power_handle.c | 96 +--
.../gpu/drm/msm/disp/dpu1/dpu_power_handle.h | 86 +
4 files changed, 6 insertions(+), 199 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 985c855796ae..23094d108e81 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -676,11 +676,6 @@ static void _dpu_kms_hw_destroy(struct dpu_kms *dpu_kms)
dpu_hw_catalog_deinit(dpu_kms->catalog);
dpu_kms->catalog = NULL;
- if (dpu_kms->core_client)
- dpu_power_client_destroy(_kms->phandle,
- dpu_kms->core_client);
- dpu_kms->core_client = NULL;
-
if (dpu_kms->vbif[VBIF_NRT])
devm_iounmap(_kms->pdev->dev, dpu_kms->vbif[VBIF_NRT]);
dpu_kms->vbif[VBIF_NRT] = NULL;
@@ -913,17 +908,6 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
dpu_kms->reg_dma_len = dpu_iomap_size(dpu_kms->pdev, "regdma");
}
- dpu_kms->core_client = dpu_power_client_create(_kms->phandle,
- "core");
- if (IS_ERR_OR_NULL(dpu_kms->core_client)) {
- rc = PTR_ERR(dpu_kms->core_client);
- if (!dpu_kms->core_client)
- rc = -EINVAL;
- DPU_ERROR("dpu power client create failed: %d\n", rc);
- dpu_kms->core_client = NULL;
- goto error;
- }
-
pm_runtime_get_sync(_kms->pdev->dev);
_dpu_kms_core_hw_rev_init(dpu_kms);
@@ -1157,8 +1141,7 @@ static int __maybe_unused dpu_runtime_suspend(struct
device *dev)
return rc;
}
- rc = dpu_power_resource_enable(_kms->phandle,
- dpu_kms->core_client, false);
+ rc = dpu_power_resource_enable(_kms->phandle, false);
if (rc)
DPU_ERROR("resource disable failed: %d\n", rc);
@@ -1189,8 +1172,7 @@ static int __maybe_unused dpu_runtime_resume(struct
device *dev)
return rc;
}
- rc = dpu_power_resource_enable(_kms->phandle,
- dpu_kms->core_client, true);
+ rc = dpu_power_resource_enable(_kms->phandle, true);
if (rc)
DPU_ERROR("resource enable failed: %d\n", rc);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index 2a3625eef6d3..f2c78deb0854 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -114,7 +114,6 @@ struct dpu_kms {
struct dpu_mdss_cfg *catalog;
struct dpu_power_handle phandle;
- struct dpu_power_client *core_client;
struct dpu_power_event *power_event;
/* directory entry for debugfs */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
index 8c6f92aaaf87..8e64f0a52147 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
@@ -35,59 +35,11 @@ static void dpu_power_event_trigger_locked(struct
dpu_power_handle *phandle,
}
}
-struct dpu_power_client *dpu_power_client_create(
- struct dpu_power_handle *phandle, char *client_name)
-{
- struct dpu_power_client *client;
- static u32 id;
-
- if (!client_name || !phandle) {
- pr_err("client name is null or invalid power data\n");
- return ERR_PTR(-EINVAL);
- }
-
- client = kzalloc(sizeof(struct dpu_power_client), GFP_KERNEL);
- if (!client)
- return ERR_PTR(-ENOMEM);
-
- mutex_lock(>phandle_lock);
- strlcpy(client->name, client_name, MAX_CLIENT_NAME_LEN);
- client->usecase_ndx = VOTE_INDEX_DISABLE;
- client->id = id;
- client->active = true;
- pr_debug("client %s created:%pK id :%d\n", client_name,
- client, id);
- id++;
- list_add(>list, >power_client_clist);
- mutex_unlock(>phandle_lock);
-
- return client;
-}
-
-void dpu_power_client_destroy(struct dpu_power_handle *phandle,
- struct dpu_power_client *client)
-{
- if (!client || !phandle) {
- pr_err("reg bus vote: invalid client handle\n");
- } else if (!client->active) {
- pr_err("dpu power deinit already done\n");
- kfree(client);
- } else {
- pr_debug("bus vote client %s destroyed:%pK id:%u\n",
- client->name, client, client->id);
- mutex_lock(>phandle_lock);
-
[Freedreno] [PATCH v2 00/24] drm/msm: Various dpu locking and legacy cleanups
From: Sean Paul This was originally 3 patchsets, but none have gotten full review, so I figured I'd package the v2's all up into one set so it's easier to track. Set 1- https://lists.freedesktop.org/archives/dri-devel/2018-November/196170.html Set 2- https://lists.freedesktop.org/archives/dri-devel/2018-November/196184.html Set 3- https://lists.freedesktop.org/archives/dri-devel/2018-November/196276.html Thanks, Sean Sean Paul (24): drm/msm: dpu: Remove dpu_power_handle_get_dbus_name() drm/msm: dpu: Remove unused trace_dpu_perf_update_bus() drm/msm: dpu: Remove dpu_power_client drm/msm: dpu: Don't use power_event for vbif_init_memtypes drm/msm: dpu: Handle crtc pm_runtime_resume() directly drm/msm: dpu: Remove power_handle from core_perf drm/msm: dpu: Include dpu_io_util.h directly in dpu_kms.h drm/msm: dpu: Move DPU_POWER_HANDLE_DBUS_ID to core_perf drm/msm: dpu: Remove dpu_power_handle drm/msm: dpu: Fix typo in dpu_encoder drm/msm: dpu: Add ->enabled to dpu_encoder_virt drm/msm: dpu: Move crtc runtime resume to encoder drm/msm: dpu: Don't drop locks in crtc_vblank_enable drm/msm: dpu: Grab the modeset locks in frame_event drm/msm: dpu: Stop using encoder->crtc pointer drm/msm: dpu: Add modeset lock checks where applicable drm/msm: dpu: Move pm_runtime_(get|put) from vblank_enable drm/msm: dpu: Remove crtc_lock from setup_mixers drm/msm: dpu: Remove vblank_callback from encoder drm/msm: dpu: Use atomic_disable for dpu_crtc_disable drm/msm: dpu: Don't bother checking ->enabled in dpu_crtc_vblank drm/msm: dpu: Separate crtc assignment from vblank enable drm/msm: dpu: Remove vblank_requested flag from dpu_crtc drm/msm: dpu: Remove crtc_lock drivers/gpu/drm/msm/Makefile | 1 - drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 37 ++- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h | 22 +- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 216 +--- drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h | 15 +- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 97 --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 24 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 76 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 6 +- .../gpu/drm/msm/disp/dpu1/dpu_power_handle.c | 240 -- .../gpu/drm/msm/disp/dpu1/dpu_power_handle.h | 217 drivers/gpu/drm/msm/disp/dpu1/dpu_trace.h | 43 +--- 12 files changed, 204 insertions(+), 790 deletions(-) delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c delete mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h -- Sean Paul, Software Engineer, Google / Chromium OS ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v2 07/24] drm/msm: dpu: Include dpu_io_util.h directly in dpu_kms.h
From: Sean Paul It's needed for struct dss_module_power, and is currently being pulled in by dpu_power_handle.h Changes in v2: - None Reviewed-by: Jeykumar Sankaran Signed-off-by: Sean Paul --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h index 5f08be187c86..4e5acacb3065 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h @@ -29,6 +29,7 @@ #include "dpu_hw_lm.h" #include "dpu_hw_interrupts.h" #include "dpu_hw_top.h" +#include "dpu_io_util.h" #include "dpu_rm.h" #include "dpu_power_handle.h" #include "dpu_irq.h" -- Sean Paul, Software Engineer, Google / Chromium OS ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v2 04/24] drm/msm: dpu: Don't use power_event for vbif_init_memtypes
From: Sean Paul
power_events are only used for pm_runtime, and that's all handled in
dpu_kms. So just call vbif_init_memtypes at the correct times.
Changes in v2:
- Removed obsolete comment (Jeykumar)
Cc: Jeykumar Sankaran
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 24 +++-
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 1 -
2 files changed, 3 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 23094d108e81..ab8574ab8327 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -651,10 +651,6 @@ static void _dpu_kms_hw_destroy(struct dpu_kms *dpu_kms)
dpu_hw_intr_destroy(dpu_kms->hw_intr);
dpu_kms->hw_intr = NULL;
- if (dpu_kms->power_event)
- dpu_power_handle_unregister_event(
- _kms->phandle, dpu_kms->power_event);
-
/* safe to call these more than once during shutdown */
_dpu_debugfs_destroy(dpu_kms);
_dpu_kms_mmu_destroy(dpu_kms);
@@ -832,16 +828,6 @@ u64 dpu_kms_get_clk_rate(struct dpu_kms *dpu_kms, char
*clock_name)
return clk_get_rate(clk->clk);
}
-static void dpu_kms_handle_power_event(u32 event_type, void *usr)
-{
- struct dpu_kms *dpu_kms = usr;
-
- if (!dpu_kms)
- return;
-
- dpu_vbif_init_memtypes(dpu_kms);
-}
-
static int dpu_kms_hw_init(struct msm_kms *kms)
{
struct dpu_kms *dpu_kms;
@@ -1012,13 +998,7 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
*/
dev->mode_config.allow_fb_modifiers = true;
- /*
-* Handle (re)initializations during power enable
-*/
- dpu_kms_handle_power_event(DPU_POWER_EVENT_ENABLE, dpu_kms);
- dpu_kms->power_event = dpu_power_handle_register_event(
- _kms->phandle, DPU_POWER_EVENT_ENABLE,
- dpu_kms_handle_power_event, dpu_kms, "kms");
+ dpu_vbif_init_memtypes(dpu_kms);
pm_runtime_put_sync(_kms->pdev->dev);
@@ -1172,6 +1152,8 @@ static int __maybe_unused dpu_runtime_resume(struct
device *dev)
return rc;
}
+ dpu_vbif_init_memtypes(dpu_kms);
+
rc = dpu_power_resource_enable(_kms->phandle, true);
if (rc)
DPU_ERROR("resource enable failed: %d\n", rc);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
index f2c78deb0854..5f08be187c86 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h
@@ -114,7 +114,6 @@ struct dpu_kms {
struct dpu_mdss_cfg *catalog;
struct dpu_power_handle phandle;
- struct dpu_power_event *power_event;
/* directory entry for debugfs */
struct dentry *debugfs_root;
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v2 06/24] drm/msm: dpu: Remove power_handle from core_perf
From: Sean Paul
It's unused
Changes in v2:
- None
Reviewed-by: Jeykumar Sankaran
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 3 ---
drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h | 5 -
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 -
3 files changed, 9 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
index 22e84b3d7f98..ef6dd43f8bec 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
@@ -605,7 +605,6 @@ void dpu_core_perf_destroy(struct dpu_core_perf *perf)
dpu_core_perf_debugfs_destroy(perf);
perf->max_core_clk_rate = 0;
perf->core_clk = NULL;
- perf->phandle = NULL;
perf->catalog = NULL;
perf->dev = NULL;
}
@@ -613,12 +612,10 @@ void dpu_core_perf_destroy(struct dpu_core_perf *perf)
int dpu_core_perf_init(struct dpu_core_perf *perf,
struct drm_device *dev,
struct dpu_mdss_cfg *catalog,
- struct dpu_power_handle *phandle,
struct dss_clk *core_clk)
{
perf->dev = dev;
perf->catalog = catalog;
- perf->phandle = phandle;
perf->core_clk = core_clk;
perf->max_core_clk_rate = core_clk->max_rate;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
index fbcbe0c7527a..68b84d85eb8f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
@@ -19,7 +19,6 @@
#include
#include "dpu_hw_catalog.h"
-#include "dpu_power_handle.h"
#defineDPU_PERF_DEFAULT_MAX_CORE_CLK_RATE 41250
@@ -52,7 +51,6 @@ struct dpu_core_perf_tune {
* @dev: Pointer to drm device
* @debugfs_root: top level debug folder
* @catalog: Pointer to catalog configuration
- * @phandle: Pointer to power handler
* @core_clk: Pointer to core clock structure
* @core_clk_rate: current core clock rate
* @max_core_clk_rate: maximum allowable core clock rate
@@ -66,7 +64,6 @@ struct dpu_core_perf {
struct drm_device *dev;
struct dentry *debugfs_root;
struct dpu_mdss_cfg *catalog;
- struct dpu_power_handle *phandle;
struct dss_clk *core_clk;
u64 core_clk_rate;
u64 max_core_clk_rate;
@@ -113,13 +110,11 @@ void dpu_core_perf_destroy(struct dpu_core_perf *perf);
* @perf: Pointer to core performance context
* @dev: Pointer to drm device
* @catalog: Pointer to catalog
- * @phandle: Pointer to power handle
* @core_clk: pointer to core clock
*/
int dpu_core_perf_init(struct dpu_core_perf *perf,
struct drm_device *dev,
struct dpu_mdss_cfg *catalog,
- struct dpu_power_handle *phandle,
struct dss_clk *core_clk);
/**
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 654ea5060e02..af666d917a0b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -957,7 +957,6 @@ static int dpu_kms_hw_init(struct msm_kms *kms)
}
rc = dpu_core_perf_init(_kms->perf, dev, dpu_kms->catalog,
- _kms->phandle,
_dpu_kms_get_clk(dpu_kms, "core"));
if (rc) {
DPU_ERROR("failed to init perf %d\n", rc);
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [PATCH v2 01/24] drm/msm: dpu: Remove dpu_power_handle_get_dbus_name()
From: Sean Paul
It's only used for debugfs, so just output the enum value instead.
Changes in v2:
- None
Reviewed-by: Jeykumar Sankaran
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 6 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c | 14 --
drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h | 7 ---
3 files changed, 2 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index ed84cf44a222..e09209d6c469 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1381,11 +1381,9 @@ static int dpu_crtc_debugfs_state_show(struct seq_file
*s, void *v)
dpu_crtc->cur_perf.core_clk_rate);
for (i = DPU_POWER_HANDLE_DBUS_ID_MNOC;
i < DPU_POWER_HANDLE_DBUS_ID_MAX; i++) {
- seq_printf(s, "bw_ctl[%s]: %llu\n",
- dpu_power_handle_get_dbus_name(i),
+ seq_printf(s, "bw_ctl[%d]: %llu\n", i,
dpu_crtc->cur_perf.bw_ctl[i]);
- seq_printf(s, "max_per_pipe_ib[%s]: %llu\n",
- dpu_power_handle_get_dbus_name(i),
+ seq_printf(s, "max_per_pipe_ib[%d]: %llu\n", i,
dpu_crtc->cur_perf.max_per_pipe_ib[i]);
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
index fc14116789f2..8c6f92aaaf87 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.c
@@ -24,20 +24,6 @@
#include "dpu_power_handle.h"
#include "dpu_trace.h"
-static const char *data_bus_name[DPU_POWER_HANDLE_DBUS_ID_MAX] = {
- [DPU_POWER_HANDLE_DBUS_ID_MNOC] = "qcom,dpu-data-bus",
- [DPU_POWER_HANDLE_DBUS_ID_LLCC] = "qcom,dpu-llcc-bus",
- [DPU_POWER_HANDLE_DBUS_ID_EBI] = "qcom,dpu-ebi-bus",
-};
-
-const char *dpu_power_handle_get_dbus_name(u32 bus_id)
-{
- if (bus_id < DPU_POWER_HANDLE_DBUS_ID_MAX)
- return data_bus_name[bus_id];
-
- return NULL;
-}
-
static void dpu_power_event_trigger_locked(struct dpu_power_handle *phandle,
u32 event_type)
{
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
index a65b7a297f21..f627ae28ec68 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_power_handle.h
@@ -207,11 +207,4 @@ struct dpu_power_event *dpu_power_handle_register_event(
void dpu_power_handle_unregister_event(struct dpu_power_handle *phandle,
struct dpu_power_event *event);
-/**
- * dpu_power_handle_get_dbus_name - get name of given data bus identifier
- * @bus_id:data bus identifier
- * Return: Pointer to name string if success; NULL otherwise
- */
-const char *dpu_power_handle_get_dbus_name(u32 bus_id);
-
#endif /* _DPU_POWER_HANDLE_H_ */
--
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
Re: [Freedreno] [PATCH 3/8] drm/msm: dpu: Remove vblank_callback from encoder
On Wed, Nov 14, 2018 at 02:43:51PM -0500, Sean Paul wrote:
> On Wed, Nov 14, 2018 at 11:33:53AM -0800, Jeykumar Sankaran wrote:
> > On 2018-11-14 07:13, Sean Paul wrote:
> > > On Tue, Nov 13, 2018 at 04:47:22PM -0800, Jeykumar Sankaran wrote:
> > > > On 2018-11-13 12:52, Sean Paul wrote:
> > > > > From: Sean Paul
> > > > >
> > > > > The indirection of registering a callback and opaque pointer isn't
> > > real
> > > > > useful when there's only one callsite. So instead of having the
> > > > > vblank_cb registration, just give encoder a crtc and let it directly
> > > > > call the vblank handler.
> > > > >
> > > > > In a later patch, we'll make use of this further.
> > > > >
> > > > > Signed-off-by: Sean Paul
> > > > > ---
> > > > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 8 +++
> > > > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h| 6 +
> > > > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 25
> > > +++--
> > > > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 10 -
> > > > > 4 files changed, 26 insertions(+), 23 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > > > index adda0aa0cbaa..38119b4d4a80 100644
> > > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > > > @@ -291,9 +291,8 @@ enum dpu_intf_mode dpu_crtc_get_intf_mode(struct
> > > > > drm_crtc *crtc)
> > > > > return INTF_MODE_NONE;
> > > > > }
> > > > >
> > > > > -static void dpu_crtc_vblank_cb(void *data)
> > > > > +void dpu_crtc_vblank_callback(struct drm_crtc *crtc)
> > > > > {
> > > > > - struct drm_crtc *crtc = (struct drm_crtc *)data;
> > > > > struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
> > > >
> > > > Since we are calling into a locally stored CRTC and not tracking
> > > disable.
> > > > Should
> > > > we check for crtc->enable before processing the callback?
> > > >
> > > > I see vblank_on/off cant be called when CRTC is disabled with the rest
> > > > of the patches in the series. But this callback is triggered by IRQ's
> > > > context.
> > >
> > > Hmm, yeah, I had assumed that we wouldn't have any interrupts after irq
> > > disable.
> > > However it doesn't seem like that's the case.
> > >
> > > That said, I don't think checking crtc->enable is quite what we want. In
> > > the
> > > case of disable, the crtc is cleared from the encoder, so checking for
> > > crtc != NULL would be better here. SGTY?
> > That would still cause a race as crtc assignment will be protected by
> > modeset
> > locks and crtc != NULL check isn't.
>
> Right, we'd need to wrap the callback in the encoder spinlock.
Just went to do this, and it's already wrapped in the encoder spinlock. So
AFAICT, no further changes needed.
Sean
>
> > >
> > > Now that I've dug into the vblank irq handling a bit in the encoder, it
> > > seems
> > > like that could be moved to the crtc and a bunch of the
> > > encoder->crtc->encoder
> > > bouncing around could be avoided. Off the top of your head, is there any
> > > reason
> > > we couldn't move the vblank irq handling into crtc from encoder?
> > vblank irq handlers are only needed for video mode. As with all the other
> > IRQ's, the handlers are implemented in phys_encoder level and the events
> > are forwarded as and when needed.
> >
>
> I took a look at how that might work, and it seems possible but probably not
> worthwhile.
>
> Sean
>
> > BTW, the vblank irq flows from phys_enc->virtual_enc->crtc.
> >
> > Thanks,
> > Jeykumar S.
> >
> > >
> > > Sean
> > >
> > > >
> > > > >
> > > > > /* keep statistics on vblank callback - with auto reset via
> > > > > debugfs */
> > > > > @@ -779,8 +778,7 @@ static void _dpu_crtc_vblank_enable_no_lock(
> > > > >DRMID(enc), enable,
> > > > >dpu_crtc);
> > > > >
> > > > > - dpu_encoder_register_vblank_callback(enc,
> > > > > - dpu_crtc_vblank_cb, (void
> > > > > *)crtc);
> > > > > + dpu_encoder_assign_crtc(enc, crtc);
> > > > > }
> > > > > } else {
> > > > > list_for_each_entry(enc, >mode_config.encoder_list,
> > > > > head) {
> > > > > @@ -791,7 +789,7 @@ static void _dpu_crtc_vblank_enable_no_lock(
> > > > >DRMID(enc), enable,
> > > > >dpu_crtc);
> > > > >
> > > > > - dpu_encoder_register_vblank_callback(enc, NULL,
> > > > > NULL);
> > > > > + dpu_encoder_assign_crtc(enc, NULL);
> > > > > }
> > > > > }
> > > > > }
> > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> > > > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h
> > > > > index 93d21a61a040..54595cc29be5 100644
[Freedreno] [PATCH -next] drm/msm: dpu: Fix "WARNING: invalid free of devm_ allocated data"
'dpu_enc' is a member of 'drm_enc' And 'drm_enc' got allocated with devm_kzalloc in dpu_encoder_init. This gives this error message: ./drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c:459:1-6: WARNING: invalid free of devm_ allocated data Signed-off-by: YueHaibing --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index 82c55ef..99526d9 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -455,8 +455,6 @@ static void dpu_encoder_destroy(struct drm_encoder *drm_enc) drm_encoder_cleanup(drm_enc); mutex_destroy(_enc->enc_lock); - - kfree(dpu_enc); } void dpu_encoder_helper_split_config( -- 2.7.0 ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [RESEND PATCH v17 5/5] iommu/arm-smmu: Add support for qcom, smmu-v2 variant
qcom,smmu-v2 is an arm,smmu-v2 implementation with specific
clock and power requirements.
On msm8996, multiple cores, viz. mdss, video, etc. use this
smmu. On sdm845, this smmu is used with gpu.
Add bindings for the same.
Signed-off-by: Vivek Gautam
Reviewed-by: Rob Herring
Reviewed-by: Tomasz Figa
Tested-by: Srinivas Kandagatla
Reviewed-by: Robin Murphy
---
drivers/iommu/arm-smmu.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 2098c3141f5f..d315ca637097 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -120,6 +120,7 @@ enum arm_smmu_implementation {
GENERIC_SMMU,
ARM_MMU500,
CAVIUM_SMMUV2,
+ QCOM_SMMUV2,
};
struct arm_smmu_s2cr {
@@ -2026,6 +2027,17 @@ ARM_SMMU_MATCH_DATA(arm_mmu401, ARM_SMMU_V1_64K,
GENERIC_SMMU);
ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500);
ARM_SMMU_MATCH_DATA(cavium_smmuv2, ARM_SMMU_V2, CAVIUM_SMMUV2);
+static const char * const qcom_smmuv2_clks[] = {
+ "bus", "iface",
+};
+
+static const struct arm_smmu_match_data qcom_smmuv2 = {
+ .version = ARM_SMMU_V2,
+ .model = QCOM_SMMUV2,
+ .clks = qcom_smmuv2_clks,
+ .num_clks = ARRAY_SIZE(qcom_smmuv2_clks),
+};
+
static const struct of_device_id arm_smmu_of_match[] = {
{ .compatible = "arm,smmu-v1", .data = _generic_v1 },
{ .compatible = "arm,smmu-v2", .data = _generic_v2 },
@@ -2033,6 +2045,7 @@ static const struct of_device_id arm_smmu_of_match[] = {
{ .compatible = "arm,mmu-401", .data = _mmu401 },
{ .compatible = "arm,mmu-500", .data = _mmu500 },
{ .compatible = "cavium,smmu-v2", .data = _smmuv2 },
+ { .compatible = "qcom,smmu-v2", .data = _smmuv2 },
{ },
};
MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [RESEND PATCH v17 3/5] iommu/arm-smmu: Add the device_link between masters and smmu
From: Sricharan R Finally add the device link between the master device and smmu, so that the smmu gets runtime enabled/disabled only when the master needs it. This is done from add_device callback which gets called once when the master is added to the smmu. Signed-off-by: Sricharan R Signed-off-by: Vivek Gautam Reviewed-by: Tomasz Figa Tested-by: Srinivas Kandagatla Reviewed-by: Robin Murphy --- drivers/iommu/arm-smmu.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index cae88c9f83ca..2098c3141f5f 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -1493,6 +1493,9 @@ static int arm_smmu_add_device(struct device *dev) iommu_device_link(>iommu, dev); + device_link_add(dev, smmu->dev, + DL_FLAG_PM_RUNTIME | DL_FLAG_AUTOREMOVE_SUPPLIER); + return 0; out_cfg_free: -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [RESEND PATCH v17 4/5] dt-bindings: arm-smmu: Add bindings for qcom, smmu-v2
Add bindings doc for Qcom's smmu-v2 implementation.
Signed-off-by: Vivek Gautam
Reviewed-by: Tomasz Figa
Tested-by: Srinivas Kandagatla
Reviewed-by: Rob Herring
Reviewed-by: Robin Murphy
---
.../devicetree/bindings/iommu/arm,smmu.txt | 39 ++
1 file changed, 39 insertions(+)
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
index 8a6ffce12af5..a6504b37cc21 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
@@ -17,10 +17,16 @@ conditions.
"arm,mmu-401"
"arm,mmu-500"
"cavium,smmu-v2"
+"qcom,smmu-v2"
depending on the particular implementation and/or the
version of the architecture implemented.
+ Qcom SoCs must contain, as below, SoC-specific compatibles
+ along with "qcom,smmu-v2":
+ "qcom,msm8996-smmu-v2", "qcom,smmu-v2",
+ "qcom,sdm845-smmu-v2", "qcom,smmu-v2".
+
- reg : Base address and size of the SMMU.
- #global-interrupts : The number of global interrupts exposed by the
@@ -71,6 +77,22 @@ conditions.
or using stream matching with #iommu-cells = <2>, and
may be ignored if present in such cases.
+- clock-names:List of the names of clocks input to the device. The
+ required list depends on particular implementation and
+ is as follows:
+ - for "qcom,smmu-v2":
+- "bus": clock required for downstream bus access and
+ for the smmu ptw,
+- "iface": clock required to access smmu's registers
+ through the TCU's programming interface.
+ - unspecified for other implementations.
+
+- clocks: Specifiers for all clocks listed in the clock-names property,
+ as per generic clock bindings.
+
+- power-domains: Specifiers for power domains required to be powered on for
+ the SMMU to operate, as per generic power domain bindings.
+
** Deprecated properties:
- mmu-masters (deprecated in favour of the generic "iommus" binding) :
@@ -137,3 +159,20 @@ conditions.
iommu-map = <0 0 0x400>;
...
};
+
+ /* Qcom's arm,smmu-v2 implementation */
+ smmu4: iommu@d0 {
+ compatible = "qcom,msm8996-smmu-v2", "qcom,smmu-v2";
+ reg = <0xd0 0x1>;
+
+ #global-interrupts = <1>;
+ interrupts = ,
+,
+;
+ #iommu-cells = <1>;
+ power-domains = < MDSS_GDSC>;
+
+ clocks = < SMMU_MDP_AXI_CLK>,
+< SMMU_MDP_AHB_CLK>;
+ clock-names = "bus", "iface";
+ };
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
[Freedreno] [RESEND PATCH v17 2/5] iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device
From: Sricharan R
The smmu device probe/remove and add/remove master device callbacks
gets called when the smmu is not linked to its master, that is without
the context of the master device. So calling runtime apis in those places
separately.
Global locks are also initialized before enabling runtime pm as the
runtime_resume() calls device_reset() which does tlb_sync_global()
that ultimately requires locks to be initialized.
Signed-off-by: Sricharan R
[vivek: Cleanup pm runtime calls]
Signed-off-by: Vivek Gautam
Reviewed-by: Tomasz Figa
Tested-by: Srinivas Kandagatla
Reviewed-by: Robin Murphy
---
drivers/iommu/arm-smmu.c | 101 ++-
1 file changed, 91 insertions(+), 10 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index f7ab7ce87a94..cae88c9f83ca 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -270,6 +270,20 @@ static struct arm_smmu_option_prop arm_smmu_options[] = {
{ 0, NULL},
};
+static inline int arm_smmu_rpm_get(struct arm_smmu_device *smmu)
+{
+ if (pm_runtime_enabled(smmu->dev))
+ return pm_runtime_get_sync(smmu->dev);
+
+ return 0;
+}
+
+static inline void arm_smmu_rpm_put(struct arm_smmu_device *smmu)
+{
+ if (pm_runtime_enabled(smmu->dev))
+ pm_runtime_put(smmu->dev);
+}
+
static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
{
return container_of(dom, struct arm_smmu_domain, domain);
@@ -929,11 +943,15 @@ static void arm_smmu_destroy_domain_context(struct
iommu_domain *domain)
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
struct arm_smmu_device *smmu = smmu_domain->smmu;
struct arm_smmu_cfg *cfg = _domain->cfg;
- int irq;
+ int ret, irq;
if (!smmu || domain->type == IOMMU_DOMAIN_IDENTITY)
return;
+ ret = arm_smmu_rpm_get(smmu);
+ if (ret < 0)
+ return;
+
/*
* Disable the context bank and free the page tables before freeing
* it.
@@ -948,6 +966,8 @@ static void arm_smmu_destroy_domain_context(struct
iommu_domain *domain)
free_io_pgtable_ops(smmu_domain->pgtbl_ops);
__arm_smmu_free_bitmap(smmu->context_map, cfg->cbndx);
+
+ arm_smmu_rpm_put(smmu);
}
static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
@@ -1229,10 +1249,15 @@ static int arm_smmu_attach_dev(struct iommu_domain
*domain, struct device *dev)
return -ENODEV;
smmu = fwspec_smmu(fwspec);
+
+ ret = arm_smmu_rpm_get(smmu);
+ if (ret < 0)
+ return ret;
+
/* Ensure that the domain is finalised */
ret = arm_smmu_init_domain_context(domain, smmu);
if (ret < 0)
- return ret;
+ goto rpm_put;
/*
* Sanity check the domain. We don't support domains across
@@ -1242,49 +1267,74 @@ static int arm_smmu_attach_dev(struct iommu_domain
*domain, struct device *dev)
dev_err(dev,
"cannot attach to SMMU %s whilst already attached to
domain on SMMU %s\n",
dev_name(smmu_domain->smmu->dev), dev_name(smmu->dev));
- return -EINVAL;
+ ret = -EINVAL;
+ goto rpm_put;
}
/* Looks ok, so add the device to the domain */
- return arm_smmu_domain_add_master(smmu_domain, fwspec);
+ ret = arm_smmu_domain_add_master(smmu_domain, fwspec);
+
+rpm_put:
+ arm_smmu_rpm_put(smmu);
+ return ret;
}
static int arm_smmu_map(struct iommu_domain *domain, unsigned long iova,
phys_addr_t paddr, size_t size, int prot)
{
struct io_pgtable_ops *ops = to_smmu_domain(domain)->pgtbl_ops;
+ struct arm_smmu_device *smmu = to_smmu_domain(domain)->smmu;
+ int ret;
if (!ops)
return -ENODEV;
- return ops->map(ops, iova, paddr, size, prot);
+ arm_smmu_rpm_get(smmu);
+ ret = ops->map(ops, iova, paddr, size, prot);
+ arm_smmu_rpm_put(smmu);
+
+ return ret;
}
static size_t arm_smmu_unmap(struct iommu_domain *domain, unsigned long iova,
size_t size)
{
struct io_pgtable_ops *ops = to_smmu_domain(domain)->pgtbl_ops;
+ struct arm_smmu_device *smmu = to_smmu_domain(domain)->smmu;
+ size_t ret;
if (!ops)
return 0;
- return ops->unmap(ops, iova, size);
+ arm_smmu_rpm_get(smmu);
+ ret = ops->unmap(ops, iova, size);
+ arm_smmu_rpm_put(smmu);
+
+ return ret;
}
static void arm_smmu_flush_iotlb_all(struct iommu_domain *domain)
{
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+ struct arm_smmu_device *smmu = smmu_domain->smmu;
- if (smmu_domain->tlb_ops)
+ if (smmu_domain->tlb_ops) {
+ arm_smmu_rpm_get(smmu);
[Freedreno] [RESEND PATCH v17 1/5] iommu/arm-smmu: Add pm_runtime/sleep ops
From: Sricharan R
The smmu needs to be functional only when the respective
master's using it are active. The device_link feature
helps to track such functional dependencies, so that the
iommu gets powered when the master device enables itself
using pm_runtime. So by adapting the smmu driver for
runtime pm, above said dependency can be addressed.
This patch adds the pm runtime/sleep callbacks to the
driver and also the functions to parse the smmu clocks
from DT and enable them in resume/suspend.
Also, while we enable the runtime pm add a pm sleep suspend
callback that pushes devices to low power state by turning
the clocks off in a system sleep.
Also add corresponding clock enable path in resume callback.
Signed-off-by: Sricharan R
Signed-off-by: Archit Taneja
[vivek: rework for clock and pm ops]
Signed-off-by: Vivek Gautam
Reviewed-by: Tomasz Figa
Tested-by: Srinivas Kandagatla
Reviewed-by: Robin Murphy
---
drivers/iommu/arm-smmu.c | 77 ++--
1 file changed, 74 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 5a28ae892504..f7ab7ce87a94 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -48,6 +48,7 @@
#include
#include
#include
+#include
#include
#include
@@ -206,6 +207,8 @@ struct arm_smmu_device {
u32 num_global_irqs;
u32 num_context_irqs;
unsigned int*irqs;
+ struct clk_bulk_data*clks;
+ int num_clks;
u32 cavium_id_base; /* Specific to Cavium */
@@ -1944,10 +1947,12 @@ static int arm_smmu_device_cfg_probe(struct
arm_smmu_device *smmu)
struct arm_smmu_match_data {
enum arm_smmu_arch_version version;
enum arm_smmu_implementation model;
+ const char * const *clks;
+ int num_clks;
};
#define ARM_SMMU_MATCH_DATA(name, ver, imp)\
-static struct arm_smmu_match_data name = { .version = ver, .model = imp }
+static const struct arm_smmu_match_data name = { .version = ver, .model = imp }
ARM_SMMU_MATCH_DATA(smmu_generic_v1, ARM_SMMU_V1, GENERIC_SMMU);
ARM_SMMU_MATCH_DATA(smmu_generic_v2, ARM_SMMU_V2, GENERIC_SMMU);
@@ -1966,6 +1971,23 @@ static const struct of_device_id arm_smmu_of_match[] = {
};
MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
+static void arm_smmu_fill_clk_data(struct arm_smmu_device *smmu,
+ const char * const *clks)
+{
+ int i;
+
+ if (smmu->num_clks < 1)
+ return;
+
+ smmu->clks = devm_kcalloc(smmu->dev, smmu->num_clks,
+ sizeof(*smmu->clks), GFP_KERNEL);
+ if (!smmu->clks)
+ return;
+
+ for (i = 0; i < smmu->num_clks; i++)
+ smmu->clks[i].id = clks[i];
+}
+
#ifdef CONFIG_ACPI
static int acpi_smmu_get_data(u32 model, struct arm_smmu_device *smmu)
{
@@ -2048,6 +2070,9 @@ static int arm_smmu_device_dt_probe(struct
platform_device *pdev,
data = of_device_get_match_data(dev);
smmu->version = data->version;
smmu->model = data->model;
+ smmu->num_clks = data->num_clks;
+
+ arm_smmu_fill_clk_data(smmu, data->clks);
parse_driver_options(smmu);
@@ -2150,6 +2175,14 @@ static int arm_smmu_device_probe(struct platform_device
*pdev)
smmu->irqs[i] = irq;
}
+ err = devm_clk_bulk_get(smmu->dev, smmu->num_clks, smmu->clks);
+ if (err)
+ return err;
+
+ err = clk_bulk_prepare_enable(smmu->num_clks, smmu->clks);
+ if (err)
+ return err;
+
err = arm_smmu_device_cfg_probe(smmu);
if (err)
return err;
@@ -2236,6 +2269,9 @@ static int arm_smmu_device_remove(struct platform_device
*pdev)
/* Turn the thing off */
writel(sCR0_CLIENTPD, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
+
+ clk_bulk_disable_unprepare(smmu->num_clks, smmu->clks);
+
return 0;
}
@@ -2244,15 +2280,50 @@ static void arm_smmu_device_shutdown(struct
platform_device *pdev)
arm_smmu_device_remove(pdev);
}
-static int __maybe_unused arm_smmu_pm_resume(struct device *dev)
+static int __maybe_unused arm_smmu_runtime_resume(struct device *dev)
{
struct arm_smmu_device *smmu = dev_get_drvdata(dev);
+ int ret;
+
+ ret = clk_bulk_enable(smmu->num_clks, smmu->clks);
+ if (ret)
+ return ret;
arm_smmu_device_reset(smmu);
+
return 0;
}
-static SIMPLE_DEV_PM_OPS(arm_smmu_pm_ops, NULL, arm_smmu_pm_resume);
+static int __maybe_unused arm_smmu_runtime_suspend(struct device *dev)
+{
+ struct arm_smmu_device *smmu = dev_get_drvdata(dev);
+
+ clk_bulk_disable(smmu->num_clks, smmu->clks);
+
+ return 0;
+}
+
+static int __maybe_unused arm_smmu_pm_resume(struct device *dev)
+{
+
[Freedreno] [RESEND PATCH v17 0/5] iommu/arm-smmu: Add runtime pm/sleep support
Hi Will, I am resending this series after we concluded on comments [1,2] on v16 of this patch-series, and the subsequent patch [3] was posted. Kindly merge this series. Thanks Vivek Previous version of this patch series is @ [4]. Also refer to [4] for change logs for previous versions. [1] https://lore.kernel.org/patchwork/patch/979430/ [2] https://lore.kernel.org/patchwork/patch/979433/ [3] https://lore.kernel.org/patchwork/patch/994194/ [4] https://lore.kernel.org/patchwork/cover/979429/ Sricharan R (3): iommu/arm-smmu: Add pm_runtime/sleep ops iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device iommu/arm-smmu: Add the device_link between masters and smmu Vivek Gautam (2): dt-bindings: arm-smmu: Add bindings for qcom,smmu-v2 iommu/arm-smmu: Add support for qcom,smmu-v2 variant .../devicetree/bindings/iommu/arm,smmu.txt | 39 + drivers/iommu/arm-smmu.c | 192 +++-- 2 files changed, 219 insertions(+), 12 deletions(-) -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ Freedreno mailing list Freedreno@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno
