| Message ID | 1542396142-19534-1-git-send-email-jsanka@codeaurora.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [1/2] drm/msm/dpu: add dpu encoder uninit | expand |
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 <jsanka@codeaurora.org> > --- > 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(&dpu_enc->enc_lock); > > - drm_encoder_cleanup(drm_enc); > mutex_destroy(&dpu_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 >
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 <jsanka@codeaurora.org> > > --- > > 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(&dpu_enc->enc_lock); > > > > - drm_encoder_cleanup(drm_enc); > > mutex_destroy(&dpu_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
On 2018-11-16 13:35, 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 <jsanka@codeaurora.org> >> --- >> 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(&dpu_enc->enc_lock); >> >> - drm_encoder_cleanup(drm_enc); >> mutex_destroy(&dpu_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? encoder_uninit will be called by dpu_kms in patch 2/2 in case of mode_init failures. Thanks and Regards, Jeykumar S. > >> + >> +/** >> * 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 >>
On 2018-11-16 13:37, Sean Paul wrote: > 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 <jsanka@codeaurora.org> >> > --- >> > 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(&dpu_enc->enc_lock); >> > >> > - drm_encoder_cleanup(drm_enc); >> > mutex_destroy(&dpu_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 > I still need the unint here to call drm_encoder_cleanup if the display mode_init fails. I will rebase the patch on top of https://lkml.org/lkml/2018/11/17/87 Thanks, Jeykumar S. > >> >> > + >> > +/** >> > * 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
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(&dpu_enc->enc_lock); - drm_encoder_cleanup(drm_enc); mutex_destroy(&dpu_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
Add encoder interface to release dpu encoder on mode_init failures in kms. Signed-off-by: Jeykumar Sankaran <jsanka@codeaurora.org> --- 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(-)