| Message ID | 20241005153733.109607-1-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | drm: renesas: rz-du: rzg2l_du_crtc: Fix max dot clock for DPI | expand |
Hi Biju, Thank you for the patch. On Sat, Oct 05, 2024 at 04:37:31PM +0100, Biju Das wrote: > As per the RZ/G2UL hardware manual Table 33.4 Clock List, the maximum > dot clock for the DPI interface is 83.5 MHz. Add mode_valid callback > to reject modes greater than 83.5 MHz. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > Before applying the patch: > root@smarc-rzg2ul:~# modetest -M rzg2l-du > Encoders: > id crtc type possible crtcs possible clones > 37 36 none 0x00000001 0x00000001 > > Connectors: > id encoder status name size (mm) modes encoders > 38 37 connected HDMI-A-1 520x320 30 37 > modes: > index name refresh (Hz) hdisp hss hse htot vdisp vss vse vtot) > #0 1920x1080 59.72 1920 1968 2000 2080 1080 1082 1087 1111 138000 flags: phsync, nvsync; type: preferred, driver > #1 1920x1080 60.00 1920 2008 2052 2200 1080 1084 1089 1125 148500 flags: phsync, pvsync; type: driver > #2 1920x1080 59.94 1920 2008 2052 2200 1080 1084 1089 1125 148352 flags: phsync, pvsync; type: driver > #3 1920x1080 59.94 1920 2008 2052 2200 1080 1084 1089 1125 148352 flags: phsync, pvsync; type: driver > #4 1280x1024 75.02 1280 1296 1440 1688 1024 1025 1028 1066 135000 flags: phsync, pvsync; type: driver > #5 1280x1024 60.02 1280 1328 1440 1688 1024 1025 1028 1066 108000 flags: phsync, pvsync; type: driver > #6 1152x864 75.00 1152 1216 1344 1600 864 865 868 900 108000 flags: phsync, pvsync; type: driver > #7 1280x720 60.00 1280 1390 1430 1650 720 725 730 750 74250 flags: phsync, pvsync; type: userdef, driver > #8 1280x720 59.94 1280 1390 1430 1650 720 725 730 750 74176 flags: phsync, pvsync; type: driver > #9 1280x720 50.00 1280 1720 1760 1980 720 725 730 750 74250 flags: phsync, pvsync; type: driver > ... > After applying the patch: > root@smarc-rzg2ul:~# modetest -M rzg2l-du > Encoders: > id crtc type possible crtcs possible clones > 37 36 none 0x00000001 0x00000001 > > Connectors: > id encoder status name size (mm) modes encoders > 38 37 connected HDMI-A-1 520x320 23 37 > modes: > index name refresh (Hz) hdisp hss hse htot vdisp vss vse vtot) > #0 1280x720 60.00 1280 1390 1430 1650 720 725 730 750 74250 flags: phsync, pvsync; type: driver > #1 1280x720 59.94 1280 1390 1430 1650 720 725 730 750 74176 flags: phsync, pvsync; type: driver > #2 1280x720 50.00 1280 1720 1760 1980 720 725 730 750 74250 flags: phsync, pvsync; type: driver > #3 1280x720 50.00 1280 1720 1760 1980 720 725 730 750 74250 flags: phsync, pvsync; type: driver > #4 1024x768 75.03 1024 1040 1136 1312 768 769 772 800 78750 flags: phsync, pvsync; type: driver > #5 1024x768 70.07 1024 1048 1184 1328 768 771 777 806 75000 flags: nhsync, nvsync; type: driver > #6 1024x768 60.00 1024 1048 1184 1344 768 771 777 806 65000 flags: nhsync, nvsync; type: driver > #7 1080x607 59.97 1080 1120 1232 1384 607 608 611 629 52210 flags: nhsync, pvsync; type: > #8 832x624 74.55 832 864 928 1152 624 625 628 667 57284 flags: nhsync, nvsync; type: driver > #9 800x600 75.00 800 816 896 1056 600 601 604 625 49500 flags: phsync, pvsync; type: driver > ... > --- > drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c | 43 +++++++++++++++++++ > 1 file changed, 43 insertions(+) > > diff --git a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c > index 6e7aac6219be..650a2e40caf5 100644 > --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c > +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c > @@ -248,6 +248,32 @@ static void rzg2l_du_crtc_stop(struct rzg2l_du_crtc *rcrtc) > * CRTC Functions > */ > > +static int rzg2l_du_crtc_atomic_check(struct drm_crtc *crtc, > + struct drm_atomic_state *state) > +{ > + struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, > + crtc); > + struct rzg2l_du_crtc_state *rstate = to_rzg2l_crtc_state(crtc_state); > + struct drm_encoder *encoder; > + > + /* Store the routes from the CRTC output to the DU outputs. */ > + rstate->outputs = 0; > + > + drm_for_each_encoder_mask(encoder, crtc->dev, > + crtc_state->encoder_mask) { > + struct rzg2l_du_encoder *renc; > + > + /* Skip the writeback encoder. */ > + if (encoder->encoder_type == DRM_MODE_ENCODER_VIRTUAL) > + continue; > + > + renc = to_rzg2l_encoder(encoder); > + rstate->outputs |= BIT(renc->output); > + } > + > + return 0; > +} rstate->outputs is used in rzg2l_du_start_stop() but is never initialized. It looks like adding this function is a bug fix by itself, I'd split it to a separate patch. > + > static void rzg2l_du_crtc_atomic_enable(struct drm_crtc *crtc, > struct drm_atomic_state *state) > { > @@ -295,10 +321,27 @@ static void rzg2l_du_crtc_atomic_flush(struct drm_crtc *crtc, > rzg2l_du_vsp_atomic_flush(rcrtc); > } > > +static enum drm_mode_status > +rzg2l_du_crtc_mode_valid(struct drm_crtc *crtc, > + const struct drm_display_mode *mode) > +{ > + struct rzg2l_du_crtc *rcrtc = to_rzg2l_crtc(crtc); > + struct rzg2l_du_crtc_state *rstate = > + to_rzg2l_crtc_state(rcrtc->crtc.state); I'm not sure that's the right state. When .move_valid() is called from mode_valid_path(), the state you need is I believe the new state, not the current state. The issue is that you don't have access to the new state here. Wouldn't it be simpler to implement the .mode_valid() function of drm_encoder ? Then you could check renc->output, you wouldn't have to deal with the state. > + > + if ((rstate->outputs & BIT(RZG2L_DU_OUTPUT_DPAD0)) && > + mode->clock > 83500) > + return MODE_CLOCK_HIGH; > + > + return MODE_OK; > +} > + > static const struct drm_crtc_helper_funcs crtc_helper_funcs = { > + .atomic_check = rzg2l_du_crtc_atomic_check, > .atomic_flush = rzg2l_du_crtc_atomic_flush, > .atomic_enable = rzg2l_du_crtc_atomic_enable, > .atomic_disable = rzg2l_du_crtc_atomic_disable, > + .mode_valid = rzg2l_du_crtc_mode_valid, > }; >
Hi Laurent, I missed your response. Sorry for that. > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Date: Mon, Oct 7, 2024 at 7:34 PM > Subject: Re: [PATCH] drm: renesas: rz-du: rzg2l_du_crtc: Fix max dot clock for DPI > > > Hi Biju, > > Thank you for the patch. > > On Sat, Oct 05, 2024 at 04:37:31PM +0100, Biju Das wrote: > > As per the RZ/G2UL hardware manual Table 33.4 Clock List, the maximum > > dot clock for the DPI interface is 83.5 MHz. Add mode_valid callback > > to reject modes greater than 83.5 MHz. > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > --- > > Before applying the patch: > > root@smarc-rzg2ul:~# modetest -M rzg2l-du > > Encoders: > > id crtc type possible crtcs possible clones > > 37 36 none 0x00000001 0x00000001 > > > > Connectors: > > id encoder status name size (mm) modes encoders > > 38 37 connected HDMI-A-1 520x320 30 37 > > modes: > > index name refresh (Hz) hdisp hss hse htot vdisp vss vse vtot) > > #0 1920x1080 59.72 1920 1968 2000 2080 1080 1082 1087 1111 138000 flags: phsync, nvsync; type: > preferred, driver > > #1 1920x1080 60.00 1920 2008 2052 2200 1080 1084 1089 1125 148500 flags: phsync, pvsync; type: > driver > > #2 1920x1080 59.94 1920 2008 2052 2200 1080 1084 1089 1125 148352 flags: phsync, pvsync; type: > driver > > #3 1920x1080 59.94 1920 2008 2052 2200 1080 1084 1089 1125 148352 flags: phsync, pvsync; type: > driver > > #4 1280x1024 75.02 1280 1296 1440 1688 1024 1025 1028 1066 135000 flags: phsync, pvsync; type: > driver > > #5 1280x1024 60.02 1280 1328 1440 1688 1024 1025 1028 1066 108000 flags: phsync, pvsync; type: > driver > > #6 1152x864 75.00 1152 1216 1344 1600 864 865 868 900 108000 flags: phsync, pvsync; type: driver > > #7 1280x720 60.00 1280 1390 1430 1650 720 725 730 750 74250 flags: phsync, pvsync; type: userdef, > driver > > #8 1280x720 59.94 1280 1390 1430 1650 720 725 730 750 74176 flags: phsync, pvsync; type: driver > > #9 1280x720 50.00 1280 1720 1760 1980 720 725 730 750 74250 flags: phsync, pvsync; type: driver > > ... > > After applying the patch: > > root@smarc-rzg2ul:~# modetest -M rzg2l-du > > Encoders: > > id crtc type possible crtcs possible clones > > 37 36 none 0x00000001 0x00000001 > > > > Connectors: > > id encoder status name size (mm) modes encoders > > 38 37 connected HDMI-A-1 520x320 23 37 > > modes: > > index name refresh (Hz) hdisp hss hse htot vdisp vss vse vtot) > > #0 1280x720 60.00 1280 1390 1430 1650 720 725 730 750 74250 flags: phsync, pvsync; type: driver > > #1 1280x720 59.94 1280 1390 1430 1650 720 725 730 750 74176 flags: phsync, pvsync; type: driver > > #2 1280x720 50.00 1280 1720 1760 1980 720 725 730 750 74250 flags: phsync, pvsync; type: driver > > #3 1280x720 50.00 1280 1720 1760 1980 720 725 730 750 74250 flags: phsync, pvsync; type: driver > > #4 1024x768 75.03 1024 1040 1136 1312 768 769 772 800 78750 flags: phsync, pvsync; type: driver > > #5 1024x768 70.07 1024 1048 1184 1328 768 771 777 806 75000 flags: nhsync, nvsync; type: driver > > #6 1024x768 60.00 1024 1048 1184 1344 768 771 777 806 65000 flags: nhsync, nvsync; type: driver > > #7 1080x607 59.97 1080 1120 1232 1384 607 608 611 629 52210 flags: nhsync, pvsync; type: > > #8 832x624 74.55 832 864 928 1152 624 625 628 667 57284 flags: nhsync, nvsync; type: driver > > #9 800x600 75.00 800 816 896 1056 600 601 604 625 49500 flags: phsync, pvsync; type: driver > > ... > > --- > > drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c | 43 > > +++++++++++++++++++ > > 1 file changed, 43 insertions(+) > > > > diff --git a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c > > b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c > > index 6e7aac6219be..650a2e40caf5 100644 > > --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c > > +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c > > @@ -248,6 +248,32 @@ static void rzg2l_du_crtc_stop(struct rzg2l_du_crtc *rcrtc) > > * CRTC Functions > > */ > > > > +static int rzg2l_du_crtc_atomic_check(struct drm_crtc *crtc, > > + struct drm_atomic_state *state) { > > + struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, > > + crtc); > > + struct rzg2l_du_crtc_state *rstate = to_rzg2l_crtc_state(crtc_state); > > + struct drm_encoder *encoder; > > + > > + /* Store the routes from the CRTC output to the DU outputs. */ > > + rstate->outputs = 0; > > + > > + drm_for_each_encoder_mask(encoder, crtc->dev, > > + crtc_state->encoder_mask) { > > + struct rzg2l_du_encoder *renc; > > + > > + /* Skip the writeback encoder. */ > > + if (encoder->encoder_type == DRM_MODE_ENCODER_VIRTUAL) > > + continue; > > + > > + renc = to_rzg2l_encoder(encoder); > > + rstate->outputs |= BIT(renc->output); > > + } > > + > > + return 0; > > +} > > rstate->outputs is used in rzg2l_du_start_stop() but is never > initialized. It looks like adding this function is a bug fix by itself, I'd split it to a separate > patch. Basically this is not required, because of [1] and after implementing .mode_valid() of drm_encoder. [1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20241001110141.94504-1-biju.das.jz@bp.renesas.com/ > > > + > > static void rzg2l_du_crtc_atomic_enable(struct drm_crtc *crtc, > > struct drm_atomic_state *state) > > { @@ -295,10 +321,27 @@ static void rzg2l_du_crtc_atomic_flush(struct > > drm_crtc *crtc, > > rzg2l_du_vsp_atomic_flush(rcrtc); } > > > > +static enum drm_mode_status > > +rzg2l_du_crtc_mode_valid(struct drm_crtc *crtc, > > + const struct drm_display_mode *mode) { > > + struct rzg2l_du_crtc *rcrtc = to_rzg2l_crtc(crtc); > > + struct rzg2l_du_crtc_state *rstate = > > + > > +to_rzg2l_crtc_state(rcrtc->crtc.state); > > I'm not sure that's the right state. When .move_valid() is called from mode_valid_path(), the state > you need is I believe the new state, not the current state. The issue is that you don't have access to > the new state here. > > Wouldn't it be simpler to implement the .mode_valid() function of drm_encoder ? Then you could check > renc->output, you wouldn't have to deal with the state. Agreed, will implement the .mode_valid() function of drm_encoder. Cheers, Biju
diff --git a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c index 6e7aac6219be..650a2e40caf5 100644 --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c @@ -248,6 +248,32 @@ static void rzg2l_du_crtc_stop(struct rzg2l_du_crtc *rcrtc) * CRTC Functions */ +static int rzg2l_du_crtc_atomic_check(struct drm_crtc *crtc, + struct drm_atomic_state *state) +{ + struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, + crtc); + struct rzg2l_du_crtc_state *rstate = to_rzg2l_crtc_state(crtc_state); + struct drm_encoder *encoder; + + /* Store the routes from the CRTC output to the DU outputs. */ + rstate->outputs = 0; + + drm_for_each_encoder_mask(encoder, crtc->dev, + crtc_state->encoder_mask) { + struct rzg2l_du_encoder *renc; + + /* Skip the writeback encoder. */ + if (encoder->encoder_type == DRM_MODE_ENCODER_VIRTUAL) + continue; + + renc = to_rzg2l_encoder(encoder); + rstate->outputs |= BIT(renc->output); + } + + return 0; +} + static void rzg2l_du_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_atomic_state *state) { @@ -295,10 +321,27 @@ static void rzg2l_du_crtc_atomic_flush(struct drm_crtc *crtc, rzg2l_du_vsp_atomic_flush(rcrtc); } +static enum drm_mode_status +rzg2l_du_crtc_mode_valid(struct drm_crtc *crtc, + const struct drm_display_mode *mode) +{ + struct rzg2l_du_crtc *rcrtc = to_rzg2l_crtc(crtc); + struct rzg2l_du_crtc_state *rstate = + to_rzg2l_crtc_state(rcrtc->crtc.state); + + if ((rstate->outputs & BIT(RZG2L_DU_OUTPUT_DPAD0)) && + mode->clock > 83500) + return MODE_CLOCK_HIGH; + + return MODE_OK; +} + static const struct drm_crtc_helper_funcs crtc_helper_funcs = { + .atomic_check = rzg2l_du_crtc_atomic_check, .atomic_flush = rzg2l_du_crtc_atomic_flush, .atomic_enable = rzg2l_du_crtc_atomic_enable, .atomic_disable = rzg2l_du_crtc_atomic_disable, + .mode_valid = rzg2l_du_crtc_mode_valid, }; static struct drm_crtc_state *
As per the RZ/G2UL hardware manual Table 33.4 Clock List, the maximum dot clock for the DPI interface is 83.5 MHz. Add mode_valid callback to reject modes greater than 83.5 MHz. Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- Before applying the patch: root@smarc-rzg2ul:~# modetest -M rzg2l-du Encoders: id crtc type possible crtcs possible clones 37 36 none 0x00000001 0x00000001 Connectors: id encoder status name size (mm) modes encoders 38 37 connected HDMI-A-1 520x320 30 37 modes: index name refresh (Hz) hdisp hss hse htot vdisp vss vse vtot) #0 1920x1080 59.72 1920 1968 2000 2080 1080 1082 1087 1111 138000 flags: phsync, nvsync; type: preferred, driver #1 1920x1080 60.00 1920 2008 2052 2200 1080 1084 1089 1125 148500 flags: phsync, pvsync; type: driver #2 1920x1080 59.94 1920 2008 2052 2200 1080 1084 1089 1125 148352 flags: phsync, pvsync; type: driver #3 1920x1080 59.94 1920 2008 2052 2200 1080 1084 1089 1125 148352 flags: phsync, pvsync; type: driver #4 1280x1024 75.02 1280 1296 1440 1688 1024 1025 1028 1066 135000 flags: phsync, pvsync; type: driver #5 1280x1024 60.02 1280 1328 1440 1688 1024 1025 1028 1066 108000 flags: phsync, pvsync; type: driver #6 1152x864 75.00 1152 1216 1344 1600 864 865 868 900 108000 flags: phsync, pvsync; type: driver #7 1280x720 60.00 1280 1390 1430 1650 720 725 730 750 74250 flags: phsync, pvsync; type: userdef, driver #8 1280x720 59.94 1280 1390 1430 1650 720 725 730 750 74176 flags: phsync, pvsync; type: driver #9 1280x720 50.00 1280 1720 1760 1980 720 725 730 750 74250 flags: phsync, pvsync; type: driver ... After applying the patch: root@smarc-rzg2ul:~# modetest -M rzg2l-du Encoders: id crtc type possible crtcs possible clones 37 36 none 0x00000001 0x00000001 Connectors: id encoder status name size (mm) modes encoders 38 37 connected HDMI-A-1 520x320 23 37 modes: index name refresh (Hz) hdisp hss hse htot vdisp vss vse vtot) #0 1280x720 60.00 1280 1390 1430 1650 720 725 730 750 74250 flags: phsync, pvsync; type: driver #1 1280x720 59.94 1280 1390 1430 1650 720 725 730 750 74176 flags: phsync, pvsync; type: driver #2 1280x720 50.00 1280 1720 1760 1980 720 725 730 750 74250 flags: phsync, pvsync; type: driver #3 1280x720 50.00 1280 1720 1760 1980 720 725 730 750 74250 flags: phsync, pvsync; type: driver #4 1024x768 75.03 1024 1040 1136 1312 768 769 772 800 78750 flags: phsync, pvsync; type: driver #5 1024x768 70.07 1024 1048 1184 1328 768 771 777 806 75000 flags: nhsync, nvsync; type: driver #6 1024x768 60.00 1024 1048 1184 1344 768 771 777 806 65000 flags: nhsync, nvsync; type: driver #7 1080x607 59.97 1080 1120 1232 1384 607 608 611 629 52210 flags: nhsync, pvsync; type: #8 832x624 74.55 832 864 928 1152 624 625 628 667 57284 flags: nhsync, nvsync; type: driver #9 800x600 75.00 800 816 896 1056 600 601 604 625 49500 flags: phsync, pvsync; type: driver ... --- drivers/gpu/drm/renesas/rz-du/rzg2l_du_crtc.c | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+)