[PATCH v15 3/5] drm: renesas: Add RZ/G2L DU Support

[Biju Das]

Hi Maxime Ripard,

> -----Original Message-----
> From: Biju Das <biju.das.jz at bp.renesas.com>
> Sent: Friday, December 15, 2023 2:56 PM
> Subject: RE: [PATCH v15 3/5] drm: renesas: Add RZ/G2L DU Support
> 

> > -----Original Message-----
> > From: Maxime Ripard <mripard at kernel.org>
> > Sent: Friday, December 15, 2023 2:18 PM
> > Subject: Re: [PATCH v15 3/5] drm: renesas: Add RZ/G2L DU Support
> >
> > On Fri, Dec 15, 2023 at 01:52:28PM +0000, Biju Das wrote:
> > > > > > > > > +static int rzg2l_du_crtc_enable_vblank(struct drm_crtc
> > *crtc) {
> > > > > > > > > +	struct rzg2l_du_crtc *rcrtc = to_rzg2l_crtc(crtc);
> > > > > > > > > +
> > > > > > > > > +	rcrtc->vblank_enable = true;
> > > > > > > > > +
> > > > > > > > > +	return 0;
> > > > > > > > > +}
> > > > > > > > > +
> > > > > > > > > +static void rzg2l_du_crtc_disable_vblank(struct
> > > > > > > > > +drm_crtc
> > > > > > > > > +*crtc)
> > > > {
> > > > > > > > > +	struct rzg2l_du_crtc *rcrtc = to_rzg2l_crtc(crtc);
> > > > > > > > > +
> > > > > > > > > +	rcrtc->vblank_enable = false; }
> > > > > > > >
> > > > > > > > You should enable / disable your interrupts here
> > > > > > >
> > > > > > > We don't have dedicated vblank IRQ for enabling/disabling
> > vblank.
> > > > > > >
> > > > > > > vblank is handled by vspd.
> > > > > > >
> > > > > > > vspd is directly rendering images to display,
> > > > > > > rcar_du_crtc_finish_page_flip() and drm_crtc_handle_vblank()
> > > > > > > called in vspd's pageflip context.
> > > > > > >
> > > > > > > See rzg2l_du_vsp_complete()in rzg2l_du_vsp.c
> > > > > >
> > > > > > Sorry, I couldn't really get how the interrupt flow / vblank
> > > > > > reporting is going to work. Could you explain it a bit more?
> > > > >
> > > > > We just need to handle vertical blanking in the VSP frame end
> > handler.
> > > > > See the code below.
> > > > >
> > > > > static void rzg2l_du_vsp_complete(void *private, unsigned int
> > > > > status,
> > > > > u32 crc) {
> > > > > 	struct rzg2l_du_crtc *crtc = private;
> > > > >
> > > > > 	if (crtc->vblank_enable)
> > > > > 		drm_crtc_handle_vblank(&crtc->crtc);
> > > > >
> > > > > 	if (status & VSP1_DU_STATUS_COMPLETE)
> > > > > 		rzg2l_du_crtc_finish_page_flip(crtc);
> > > > >
> > > > > 	drm_crtc_add_crc_entry(&crtc->crtc, false, 0, &crc); }
> > > >
> > > > Then we're back to the same question :)
> > > >
> > > > Why can't you mask the frame end interrupt?
> > >
> > > We are masking interrupts.
> > >
> > > [   70.639139] #######rzg2l_du_crtc_disable_vblank#######
> > > [   70.650243] #########rzg2l_du_vsp_disable ############
> > > [   70.652003] ########## vsp1_wpf_stop###
> > >
> > > Unmask is,
> > >
> > > [ 176.354520] #######rzg2l_du_crtc_enable_vblank#######
> > > [  176.354922] #########rzg2l_du_vsp_atomic_flush ############ [
> > > 176.355198] ########## wpf_configure_stream###
> >
> > Sorry, my question was why aren't you unmasking and masking them in
> > the enable/disable_vblank hooks of the CRTC.
> 
> I have n't tried that. Will try and provide feedback.
> 
> Currently the IRQ source belongs to VSPD in media subsystem.
> So I need to export an API though vsp1_drm and test it.
> 

+ linux-media

Laurent, are you ok with the below RZ/G2L specific patch[1] for enabling/disabling frame end interrupt in VSP driver?
Note:
I need to add a quirk for handling this only for RZ/G2L family as other SoCs have Vblank specific interrupt available in DU.

[1]

diff --git a/drivers/media/platform/renesas/vsp1/vsp1_drm.c b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
index 9b087bd8df7d..39347c16bb27 100644
--- a/drivers/media/platform/renesas/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_drm.c
@@ -936,6 +936,14 @@ void vsp1_du_unmap_sg(struct device *dev, struct sg_table *sgt)
 }
 EXPORT_SYMBOL_GPL(vsp1_du_unmap_sg);
 
+void vsp1_du_mask_frame_end_interrupt(struct device *dev, bool mask)
+{
+       struct vsp1_device *vsp1 = dev_get_drvdata(dev);
+
+       vsp1_write(vsp1, VI6_WPF_IRQ_ENB(0), mask ? 0 : VI6_WPF_IRQ_ENB_DFEE);
+}
+EXPORT_SYMBOL_GPL(vsp1_du_mask_frame_end_interrupt);
+
 /* -----------------------------------------------------------------------------
  * Initialization
  */
diff --git a/include/media/vsp1.h b/include/media/vsp1.h
index 48f4a5023d81..ccac48a6bdd2 100644
--- a/include/media/vsp1.h
+++ b/include/media/vsp1.h
@@ -117,4 +117,6 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned int pipe_index,
 int vsp1_du_map_sg(struct device *dev, struct sg_table *sgt);
 void vsp1_du_unmap_sg(struct device *dev, struct sg_table *sgt);
 
+void vsp1_du_mask_frame_end_interrupt(struct device *dev, bool mask);
+
 #endif /* __MEDIA_VSP1_H__ */

Cheers,
Biju