Hi Laurent,
On 23/11/2018 14:47, Laurent Pinchart wrote:
> Hi Kieran,
>
> 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?
How about keep the (corrected?) constant value, but add a comment
referencing it's extraction.
I don't think the coded table extraction helps here. Especially as it
necessitates indexing against ARRAY_SIZE - 2.
--
Regards
Kieran
>
>>> + 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
--
Kieran
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
