Re: [PATCH v4 3/4] drm/msm/dpu: Remove empty prepare_commit() function
On 3/1/2023 2:13 AM, Marijn Suijten wrote:
On 2023-03-01 11:08:16, Marijn Suijten wrote:
On 2023-02-21 10:42:55, Jessica Zhang wrote:
Now that the TE setup has been moved to prepare_for_kickoff(), we have
not prepare_commit() callbacks left. This makes dpu_encoder_prepare_commit()
s/not/no
do nothing. Remove prepare_commit() from DPU driver.
And again, this:
Changes in V3:
- Reworded commit message to be more clear
- Corrected spelling mistake in commit message
Changes in V4:
- Reworded commit message for clarity
... should go below the cut.
Signed-off-by: Jessica Zhang
With the above two issues fixed:
Reviewed-by: Marijn Suijten
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 19 ---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 7 ---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 21 -
3 files changed, 47 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index dcceed91aed8..35e120b5ef53 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2090,25 +2090,6 @@ void dpu_encoder_helper_phys_cleanup(struct
dpu_encoder_phys *phys_enc)
ctl->ops.clear_pending_flush(ctl);
}
-void dpu_encoder_prepare_commit(struct drm_encoder *drm_enc)
-{
- struct dpu_encoder_virt *dpu_enc;
- struct dpu_encoder_phys *phys;
- int i;
-
- if (!drm_enc) {
- DPU_ERROR("invalid encoder\n");
- return;
- }
- dpu_enc = to_dpu_encoder_virt(drm_enc);
-
- for (i = 0; i < dpu_enc->num_phys_encs; i++) {
- phys = dpu_enc->phys_encs[i];
- if (phys->ops.prepare_commit)
- phys->ops.prepare_commit(phys);
In hindsight, Dmitry asked in v2 to remove prepare_commit from
dpu_encoder_phys_ops (and its documentation comment) in
dpu_encoder_phys.h, but that has not happened yet. Can we do that in a
v5?
Ah, forgot to include that change. Will add it in the v5. Thanks for
catching it!
- Jessica Zhang
- Marijn
- }
-}
-
#ifdef CONFIG_DEBUG_FS
static int _dpu_encoder_status_show(struct seq_file *s, void *data)
{
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
index 9e7236ef34e6..2c9ef8d1b877 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
@@ -146,13 +146,6 @@ struct drm_encoder *dpu_encoder_init(
int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc,
struct msm_display_info *disp_info);
-/**
- * dpu_encoder_prepare_commit - prepare encoder at the very beginning of an
- * atomic commit, before any registers are written
- * @drm_enc:Pointer to previously created drm encoder structure
- */
-void dpu_encoder_prepare_commit(struct drm_encoder *drm_enc);
-
/**
* dpu_encoder_set_idle_timeout - set the idle timeout for video
*and command mode encoders.
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 165958d47ec6..6f7ddbf0d9b7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -425,26 +425,6 @@ static ktime_t dpu_kms_vsync_time(struct msm_kms *kms,
struct drm_crtc *crtc)
return ktime_get();
}
-static void dpu_kms_prepare_commit(struct msm_kms *kms,
- struct drm_atomic_state *state)
-{
- struct drm_crtc *crtc;
- struct drm_crtc_state *crtc_state;
- struct drm_encoder *encoder;
- int i;
-
- if (!kms)
- return;
-
- /* Call prepare_commit for all affected encoders */
- for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
- drm_for_each_encoder_mask(encoder, crtc->dev,
- crtc_state->encoder_mask) {
- dpu_encoder_prepare_commit(encoder);
- }
- }
-}
-
static void dpu_kms_flush_commit(struct msm_kms *kms, unsigned crtc_mask)
{
struct dpu_kms *dpu_kms = to_dpu_kms(kms);
@@ -949,7 +929,6 @@ static const struct msm_kms_funcs kms_funcs = {
.enable_commit = dpu_kms_enable_commit,
.disable_commit = dpu_kms_disable_commit,
.vsync_time = dpu_kms_vsync_time,
- .prepare_commit = dpu_kms_prepare_commit,
.flush_commit= dpu_kms_flush_commit,
.wait_flush = dpu_kms_wait_flush,
.complete_commit = dpu_kms_complete_commit,
--
2.39.2
Re: [PATCH v4 3/4] drm/msm/dpu: Remove empty prepare_commit() function
On 2023-03-01 11:08:16, Marijn Suijten wrote:
> On 2023-02-21 10:42:55, Jessica Zhang wrote:
> > Now that the TE setup has been moved to prepare_for_kickoff(), we have
> > not prepare_commit() callbacks left. This makes dpu_encoder_prepare_commit()
>
> s/not/no
>
> > do nothing. Remove prepare_commit() from DPU driver.
>
> And again, this:
>
> > Changes in V3:
> > - Reworded commit message to be more clear
> > - Corrected spelling mistake in commit message
> >
> > Changes in V4:
> > - Reworded commit message for clarity
>
> ... should go below the cut.
>
> > Signed-off-by: Jessica Zhang
>
> With the above two issues fixed:
>
> Reviewed-by: Marijn Suijten
>
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 19 ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 7 ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 21 -
> > 3 files changed, 47 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > index dcceed91aed8..35e120b5ef53 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > @@ -2090,25 +2090,6 @@ void dpu_encoder_helper_phys_cleanup(struct
> > dpu_encoder_phys *phys_enc)
> > ctl->ops.clear_pending_flush(ctl);
> > }
> >
> > -void dpu_encoder_prepare_commit(struct drm_encoder *drm_enc)
> > -{
> > - struct dpu_encoder_virt *dpu_enc;
> > - struct dpu_encoder_phys *phys;
> > - int i;
> > -
> > - if (!drm_enc) {
> > - DPU_ERROR("invalid encoder\n");
> > - return;
> > - }
> > - dpu_enc = to_dpu_encoder_virt(drm_enc);
> > -
> > - for (i = 0; i < dpu_enc->num_phys_encs; i++) {
> > - phys = dpu_enc->phys_encs[i];
> > - if (phys->ops.prepare_commit)
> > - phys->ops.prepare_commit(phys);
In hindsight, Dmitry asked in v2 to remove prepare_commit from
dpu_encoder_phys_ops (and its documentation comment) in
dpu_encoder_phys.h, but that has not happened yet. Can we do that in a
v5?
- Marijn
> > - }
> > -}
> > -
> > #ifdef CONFIG_DEBUG_FS
> > static int _dpu_encoder_status_show(struct seq_file *s, void *data)
> > {
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> > index 9e7236ef34e6..2c9ef8d1b877 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> > @@ -146,13 +146,6 @@ struct drm_encoder *dpu_encoder_init(
> > int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc,
> > struct msm_display_info *disp_info);
> >
> > -/**
> > - * dpu_encoder_prepare_commit - prepare encoder at the very beginning of an
> > - * atomic commit, before any registers are written
> > - * @drm_enc:Pointer to previously created drm encoder structure
> > - */
> > -void dpu_encoder_prepare_commit(struct drm_encoder *drm_enc);
> > -
> > /**
> > * dpu_encoder_set_idle_timeout - set the idle timeout for video
> > *and command mode encoders.
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > index 165958d47ec6..6f7ddbf0d9b7 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > @@ -425,26 +425,6 @@ static ktime_t dpu_kms_vsync_time(struct msm_kms *kms,
> > struct drm_crtc *crtc)
> > return ktime_get();
> > }
> >
> > -static void dpu_kms_prepare_commit(struct msm_kms *kms,
> > - struct drm_atomic_state *state)
> > -{
> > - struct drm_crtc *crtc;
> > - struct drm_crtc_state *crtc_state;
> > - struct drm_encoder *encoder;
> > - int i;
> > -
> > - if (!kms)
> > - return;
> > -
> > - /* Call prepare_commit for all affected encoders */
> > - for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> > - drm_for_each_encoder_mask(encoder, crtc->dev,
> > - crtc_state->encoder_mask) {
> > - dpu_encoder_prepare_commit(encoder);
> > - }
> > - }
> > -}
> > -
> > static void dpu_kms_flush_commit(struct msm_kms *kms, unsigned crtc_mask)
> > {
> > struct dpu_kms *dpu_kms = to_dpu_kms(kms);
> > @@ -949,7 +929,6 @@ static const struct msm_kms_funcs kms_funcs = {
> > .enable_commit = dpu_kms_enable_commit,
> > .disable_commit = dpu_kms_disable_commit,
> > .vsync_time = dpu_kms_vsync_time,
> > - .prepare_commit = dpu_kms_prepare_commit,
> > .flush_commit= dpu_kms_flush_commit,
> > .wait_flush = dpu_kms_wait_flush,
> > .complete_commit = dpu_kms_complete_commit,
> > --
> > 2.39.2
> >
Re: [PATCH v4 3/4] drm/msm/dpu: Remove empty prepare_commit() function
On 2023-02-21 10:42:55, Jessica Zhang wrote:
> Now that the TE setup has been moved to prepare_for_kickoff(), we have
> not prepare_commit() callbacks left. This makes dpu_encoder_prepare_commit()
s/not/no
> do nothing. Remove prepare_commit() from DPU driver.
And again, this:
> Changes in V3:
> - Reworded commit message to be more clear
> - Corrected spelling mistake in commit message
>
> Changes in V4:
> - Reworded commit message for clarity
... should go below the cut.
> Signed-off-by: Jessica Zhang
With the above two issues fixed:
Reviewed-by: Marijn Suijten
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 19 ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 7 ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 21 -
> 3 files changed, 47 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index dcceed91aed8..35e120b5ef53 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -2090,25 +2090,6 @@ void dpu_encoder_helper_phys_cleanup(struct
> dpu_encoder_phys *phys_enc)
> ctl->ops.clear_pending_flush(ctl);
> }
>
> -void dpu_encoder_prepare_commit(struct drm_encoder *drm_enc)
> -{
> - struct dpu_encoder_virt *dpu_enc;
> - struct dpu_encoder_phys *phys;
> - int i;
> -
> - if (!drm_enc) {
> - DPU_ERROR("invalid encoder\n");
> - return;
> - }
> - dpu_enc = to_dpu_encoder_virt(drm_enc);
> -
> - for (i = 0; i < dpu_enc->num_phys_encs; i++) {
> - phys = dpu_enc->phys_encs[i];
> - if (phys->ops.prepare_commit)
> - phys->ops.prepare_commit(phys);
> - }
> -}
> -
> #ifdef CONFIG_DEBUG_FS
> static int _dpu_encoder_status_show(struct seq_file *s, void *data)
> {
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> index 9e7236ef34e6..2c9ef8d1b877 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> @@ -146,13 +146,6 @@ struct drm_encoder *dpu_encoder_init(
> int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc,
> struct msm_display_info *disp_info);
>
> -/**
> - * dpu_encoder_prepare_commit - prepare encoder at the very beginning of an
> - * atomic commit, before any registers are written
> - * @drm_enc:Pointer to previously created drm encoder structure
> - */
> -void dpu_encoder_prepare_commit(struct drm_encoder *drm_enc);
> -
> /**
> * dpu_encoder_set_idle_timeout - set the idle timeout for video
> *and command mode encoders.
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 165958d47ec6..6f7ddbf0d9b7 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -425,26 +425,6 @@ static ktime_t dpu_kms_vsync_time(struct msm_kms *kms,
> struct drm_crtc *crtc)
> return ktime_get();
> }
>
> -static void dpu_kms_prepare_commit(struct msm_kms *kms,
> - struct drm_atomic_state *state)
> -{
> - struct drm_crtc *crtc;
> - struct drm_crtc_state *crtc_state;
> - struct drm_encoder *encoder;
> - int i;
> -
> - if (!kms)
> - return;
> -
> - /* Call prepare_commit for all affected encoders */
> - for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> - drm_for_each_encoder_mask(encoder, crtc->dev,
> - crtc_state->encoder_mask) {
> - dpu_encoder_prepare_commit(encoder);
> - }
> - }
> -}
> -
> static void dpu_kms_flush_commit(struct msm_kms *kms, unsigned crtc_mask)
> {
> struct dpu_kms *dpu_kms = to_dpu_kms(kms);
> @@ -949,7 +929,6 @@ static const struct msm_kms_funcs kms_funcs = {
> .enable_commit = dpu_kms_enable_commit,
> .disable_commit = dpu_kms_disable_commit,
> .vsync_time = dpu_kms_vsync_time,
> - .prepare_commit = dpu_kms_prepare_commit,
> .flush_commit= dpu_kms_flush_commit,
> .wait_flush = dpu_kms_wait_flush,
> .complete_commit = dpu_kms_complete_commit,
> --
> 2.39.2
>
Re: [PATCH v4 3/4] drm/msm/dpu: Remove empty prepare_commit() function
On 21/02/2023 20:42, Jessica Zhang wrote: Now that the TE setup has been moved to prepare_for_kickoff(), we have not prepare_commit() callbacks left. This makes dpu_encoder_prepare_commit() do nothing. Remove prepare_commit() from DPU driver. Changes in V3: - Reworded commit message to be more clear - Corrected spelling mistake in commit message Changes in V4: - Reworded commit message for clarity Signed-off-by: Jessica Zhang --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 19 --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 7 --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 21 - 3 files changed, 47 deletions(-) Reviewed-by: Dmitry Baryshkov -- With best wishes Dmitry
[PATCH v4 3/4] drm/msm/dpu: Remove empty prepare_commit() function
Now that the TE setup has been moved to prepare_for_kickoff(), we have
not prepare_commit() callbacks left. This makes dpu_encoder_prepare_commit()
do nothing. Remove prepare_commit() from DPU driver.
Changes in V3:
- Reworded commit message to be more clear
- Corrected spelling mistake in commit message
Changes in V4:
- Reworded commit message for clarity
Signed-off-by: Jessica Zhang
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 19 ---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h | 7 ---
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 21 -
3 files changed, 47 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index dcceed91aed8..35e120b5ef53 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2090,25 +2090,6 @@ void dpu_encoder_helper_phys_cleanup(struct
dpu_encoder_phys *phys_enc)
ctl->ops.clear_pending_flush(ctl);
}
-void dpu_encoder_prepare_commit(struct drm_encoder *drm_enc)
-{
- struct dpu_encoder_virt *dpu_enc;
- struct dpu_encoder_phys *phys;
- int i;
-
- if (!drm_enc) {
- DPU_ERROR("invalid encoder\n");
- return;
- }
- dpu_enc = to_dpu_encoder_virt(drm_enc);
-
- for (i = 0; i < dpu_enc->num_phys_encs; i++) {
- phys = dpu_enc->phys_encs[i];
- if (phys->ops.prepare_commit)
- phys->ops.prepare_commit(phys);
- }
-}
-
#ifdef CONFIG_DEBUG_FS
static int _dpu_encoder_status_show(struct seq_file *s, void *data)
{
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
index 9e7236ef34e6..2c9ef8d1b877 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
@@ -146,13 +146,6 @@ struct drm_encoder *dpu_encoder_init(
int dpu_encoder_setup(struct drm_device *dev, struct drm_encoder *enc,
struct msm_display_info *disp_info);
-/**
- * dpu_encoder_prepare_commit - prepare encoder at the very beginning of an
- * atomic commit, before any registers are written
- * @drm_enc:Pointer to previously created drm encoder structure
- */
-void dpu_encoder_prepare_commit(struct drm_encoder *drm_enc);
-
/**
* dpu_encoder_set_idle_timeout - set the idle timeout for video
*and command mode encoders.
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 165958d47ec6..6f7ddbf0d9b7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -425,26 +425,6 @@ static ktime_t dpu_kms_vsync_time(struct msm_kms *kms,
struct drm_crtc *crtc)
return ktime_get();
}
-static void dpu_kms_prepare_commit(struct msm_kms *kms,
- struct drm_atomic_state *state)
-{
- struct drm_crtc *crtc;
- struct drm_crtc_state *crtc_state;
- struct drm_encoder *encoder;
- int i;
-
- if (!kms)
- return;
-
- /* Call prepare_commit for all affected encoders */
- for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
- drm_for_each_encoder_mask(encoder, crtc->dev,
- crtc_state->encoder_mask) {
- dpu_encoder_prepare_commit(encoder);
- }
- }
-}
-
static void dpu_kms_flush_commit(struct msm_kms *kms, unsigned crtc_mask)
{
struct dpu_kms *dpu_kms = to_dpu_kms(kms);
@@ -949,7 +929,6 @@ static const struct msm_kms_funcs kms_funcs = {
.enable_commit = dpu_kms_enable_commit,
.disable_commit = dpu_kms_disable_commit,
.vsync_time = dpu_kms_vsync_time,
- .prepare_commit = dpu_kms_prepare_commit,
.flush_commit= dpu_kms_flush_commit,
.wait_flush = dpu_kms_wait_flush,
.complete_commit = dpu_kms_complete_commit,
--
2.39.2
