Hi Laurent, Thank you for the review.
On Wed, May 6, 2026 at 9:14 PM Laurent Pinchart <[email protected]> wrote: > > On Wed, Apr 29, 2026 at 06:00:11PM +0100, Prabhakar wrote: > > From: Lad Prabhakar <[email protected]> > > > > Move pixel clock validation from a fixed encoder check to per SoC > > constraints stored in rzg2l_du_device_info. > > > > Pixel clock limits differ across SoCs in the RZ DU family and cannot be > > expressed by a single shared rule. For example, RZ/G2UL (R9A07G043U) > > limits the DPAD0 pixel clock to 83.5 MHz, while other SoCs such as > > RZ/T2H require a wider operating range. > > > > Add mode_clock_min and mode_clock_max fields to rzg2l_du_device_info to > > describe the supported pixel clock range for each SoC. Update > > rzg2l_du_encoder_mode_valid() to return MODE_CLOCK_LOW when the pixel > > clock falls below mode_clock_min and MODE_CLOCK_HIGH when it exceeds > > mode_clock_max. > > > > Set the pixel clock limits for RZ/G2UL(R9A07G043U) to 20.875MHz minimum > > and 83.5MHz maximum. > > > > Signed-off-by: Lad Prabhakar <[email protected]> > > --- > > drivers/gpu/drm/renesas/rz-du/rzg2l_du_drv.c | 2 ++ > > drivers/gpu/drm/renesas/rz-du/rzg2l_du_drv.h | 4 ++++ > > drivers/gpu/drm/renesas/rz-du/rzg2l_du_encoder.c | 6 +++++- > > drivers/gpu/drm/renesas/rz-du/rzg2l_du_encoder.h | 2 ++ > > 4 files changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_drv.c > > b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_drv.c > > index 0fef33a5a089..3b7162c6e1f4 100644 > > --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_drv.c > > +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_drv.c > > @@ -35,6 +35,8 @@ static const struct rzg2l_du_device_info > > rzg2l_du_r9a07g043u_info = { > > .port = 0, > > }, > > }, > > + .mode_clock_min = 20875, > > + .mode_clock_max = 83500, > > }; > > > > static const struct rzg2l_du_device_info rzg2l_du_r9a07g044_info = { > > diff --git a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_drv.h > > b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_drv.h > > index 58806c2a8f2b..885558eb9547 100644 > > --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_drv.h > > +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_drv.h > > @@ -44,10 +44,14 @@ struct rzg2l_du_output_routing { > > * struct rzg2l_du_device_info - DU model-specific information > > * @channels_mask: bit mask of available DU channels > > * @routes: array of CRTC to output routes, indexed by output > > (RZG2L_DU_OUTPUT_*) > > + * @mode_clock_min: minimum pixel clock in kHz > > + * @mode_clock_max: maximum pixel clock in kHz > > */ > > struct rzg2l_du_device_info { > > unsigned int channels_mask; > > struct rzg2l_du_output_routing routes[RZG2L_DU_OUTPUT_MAX]; > > + u32 mode_clock_min; > > + u32 mode_clock_max; > > }; > > > > #define RZG2L_DU_MAX_CRTCS 1 > > diff --git a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_encoder.c > > b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_encoder.c > > index d53068733c66..ad02efec1c23 100644 > > --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_encoder.c > > +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_encoder.c > > @@ -50,8 +50,11 @@ rzg2l_du_encoder_mode_valid(struct drm_encoder *encoder, > > const struct drm_display_mode *mode) > > { > > struct rzg2l_du_encoder *renc = to_rzg2l_encoder(encoder); > > + const struct rzg2l_du_device_info *info = renc->info; > > You could use > > struct rzg2l_du_device *rcdu = to_rzg2l_du_device(renc->base.dev); > const struct rzg2l_du_device_info *info = rcdu->info; > > and avoid the info pointer in struct rzg2l_du_encoder. Up to you. > Agreed, I will drop the info pointer for now. > > > > - if (renc->output == RZG2L_DU_OUTPUT_DPAD0 && mode->clock > 83500) > > + if (info->mode_clock_min && mode->clock < info->mode_clock_min) > > + return MODE_CLOCK_LOW; > > + if (info->mode_clock_max && mode->clock > info->mode_clock_max) > > return MODE_CLOCK_HIGH; > > The new check now applies to all outputs, not just the DPAD0 output. Is > that intentional ? > The RZ/G2UL SoC only supports DPAD0 so the check is redundant. > > > > return MODE_OK; > > @@ -107,6 +110,7 @@ int rzg2l_du_encoder_init(struct rzg2l_du_device *rcdu, > > if (IS_ERR(renc)) > > return PTR_ERR(renc); > > > > + renc->info = rcdu->info; > > renc->output = output; > > drm_encoder_helper_add(&renc->base, &rzg2l_du_encoder_helper_funcs); > > > > diff --git a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_encoder.h > > b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_encoder.h > > index 3e430c1f6132..39a1d178b856 100644 > > --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_encoder.h > > +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_encoder.h > > @@ -14,10 +14,12 @@ > > #include <linux/container_of.h> > > > > struct rzg2l_du_device; > > +struct rzg2l_du_device_info; > > > > struct rzg2l_du_encoder { > > struct drm_encoder base; > > enum rzg2l_du_output output; > > + const struct rzg2l_du_device_info *info; > > If you want to keep a pointer here to avoid going through > to_rzg2l_du_device(), I would store a backpointer to rzg2l_du_device > instead of just an info pointer, it could come handy in other places. > As agreed above I will drop this pointer for now. Cheers, Prabhakar
