Hi Kieran,
On Friday, 23 November 2018 17:30:43 EET Kieran Bingham wrote:
> On 23/11/2018 14:47, Laurent Pinchart wrote:
> > On Friday, 23 November 2018 16:43:28 EET Kieran Bingham wrote:
> >> On 23/11/2018 14:34, Laurent Pinchart wrote:
> >>> Implement a .mode_valid() handler in the R-Car glue layer to reject
> >>> modes with an unsupported clock frequency.
> >>>
> >>> Signed-off-by: Laurent Pinchart
> >>> <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>
> >>> ---
> >>>
> >>> drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c | 11 +++++++++++
> >>> 1 file changed, 11 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
> >>> b/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c index
> >>> 75490a3e0a2a..8a603235f22d
> >>> 100644
> >>> --- a/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
> >>> +++ b/drivers/gpu/drm/rcar-du/rcar_dw_hdmi.c
> >>> @@ -35,6 +35,16 @@ static const struct rcar_hdmi_phy_params
> >>> rcar_hdmi_phy_params[] = {
> >>> { ~0UL, 0x0000, 0x0000, 0x0000 },
> >>> };
> >>>
> >>> +static enum drm_mode_status
> >>> +rcar_hdmi_mode_valid(struct drm_connector *connector,
> >>> + const struct drm_display_mode *mode)
> >>> +{
> >>> + if (mode->clock > 297000)
> >>
> >> Is 29700 constant? Can it be determined from any other location or is it
> >> just a magically known platform value?
> >
> > It's the last entry of the rcar_hdmi_phy_params table above. I considered
> > writing it
> >
> > if (mode->clock >
> >
> > rcar_hdmi_phy_params[ARRAY_SIZE(rcar_hdmi_phy_params)-2].mpixelclock)
> >
> > but found it a but hard to parse. Do you think it would be better ?
>
> Well - for readability - no,
>
> But for accuracy - yes:
>
> 297000000 != 297000
>
> Unless the /1000 is intentional?
I would have had to write / 1000 indeed :-) mode->clock is expressed in kHz.
> How about keep the (corrected?) constant value, but add a comment
> referencing it's extraction.
Good idea, I'll do so.
> I don't think the coded table extraction helps here. Especially as it
> necessitates indexing against ARRAY_SIZE - 2.
>
> >>> + return MODE_CLOCK_HIGH;
> >>> +
> >>> + return MODE_OK;
> >>> +}
> >>> +
> >>> static int rcar_hdmi_phy_configure(struct dw_hdmi *hdmi,
> >>> const struct dw_hdmi_plat_data *pdata,
> >>> unsigned long mpixelclock)
> >>> @@ -59,6 +69,7 @@ static int rcar_hdmi_phy_configure(struct dw_hdmi
> >>> *hdmi,
> >>> }
> >>>
> >>> static const struct dw_hdmi_plat_data rcar_dw_hdmi_plat_data = {
> >>> + .mode_valid = rcar_hdmi_mode_valid,
> >>> .configure_phy = rcar_hdmi_phy_configure,
> >>> };
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
