* [PATCH] drm: rcar-du: dw-hdmi: Reject modes with a too high clock frequency
@ 2018-11-23 14:34 ` Laurent Pinchart
0 siblings, 0 replies; 10+ messages in thread
From: Laurent Pinchart @ 2018-11-23 14:34 UTC (permalink / raw)
To: dri-devel; +Cc: linux-renesas-soc, kieran.bingham
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@ideasonboard.com>
---
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)
+ 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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] drm: rcar-du: dw-hdmi: Reject modes with a too high clock frequency
@ 2018-11-23 14:34 ` Laurent Pinchart
0 siblings, 0 replies; 10+ messages in thread
From: Laurent Pinchart @ 2018-11-23 14:34 UTC (permalink / raw)
To: dri-devel; +Cc: linux-renesas-soc, kieran.bingham
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@ideasonboard.com>
---
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)
+ 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] drm: rcar-du: dw-hdmi: Reject modes with a too high clock frequency
2018-11-23 14:34 ` Laurent Pinchart
@ 2018-11-23 14:43 ` Kieran Bingham
-1 siblings, 0 replies; 10+ messages in thread
From: Kieran Bingham @ 2018-11-23 14:43 UTC (permalink / raw)
To: Laurent Pinchart, dri-devel; +Cc: linux-renesas-soc
Hi Laurent,
Thank you for the patch,
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@ideasonboard.com>
> ---
> 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?
> + 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm: rcar-du: dw-hdmi: Reject modes with a too high clock frequency
@ 2018-11-23 14:43 ` Kieran Bingham
0 siblings, 0 replies; 10+ messages in thread
From: Kieran Bingham @ 2018-11-23 14:43 UTC (permalink / raw)
To: Laurent Pinchart, dri-devel; +Cc: linux-renesas-soc
Hi Laurent,
Thank you for the patch,
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@ideasonboard.com>
> ---
> 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?
> + 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm: rcar-du: dw-hdmi: Reject modes with a too high clock frequency
2018-11-23 14:43 ` Kieran Bingham
@ 2018-11-23 14:47 ` Laurent Pinchart
-1 siblings, 0 replies; 10+ messages in thread
From: Laurent Pinchart @ 2018-11-23 14:47 UTC (permalink / raw)
To: kieran.bingham; +Cc: Laurent Pinchart, dri-devel, linux-renesas-soc
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@ideasonboard.com>
> > ---
> >
> > 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 ?
> > + 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm: rcar-du: dw-hdmi: Reject modes with a too high clock frequency
@ 2018-11-23 14:47 ` Laurent Pinchart
0 siblings, 0 replies; 10+ messages in thread
From: Laurent Pinchart @ 2018-11-23 14:47 UTC (permalink / raw)
To: kieran.bingham; +Cc: linux-renesas-soc, Laurent Pinchart, dri-devel
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@ideasonboard.com>
> > ---
> >
> > 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 ?
> > + 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm: rcar-du: dw-hdmi: Reject modes with a too high clock frequency
2018-11-23 14:47 ` Laurent Pinchart
@ 2018-11-23 15:30 ` Kieran Bingham
-1 siblings, 0 replies; 10+ messages in thread
From: Kieran Bingham @ 2018-11-23 15:30 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: Laurent Pinchart, dri-devel, linux-renesas-soc
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@ideasonboard.com>
>>> ---
>>>
>>> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm: rcar-du: dw-hdmi: Reject modes with a too high clock frequency
@ 2018-11-23 15:30 ` Kieran Bingham
0 siblings, 0 replies; 10+ messages in thread
From: Kieran Bingham @ 2018-11-23 15:30 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-renesas-soc, Laurent Pinchart, dri-devel
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@ideasonboard.com>
>>> ---
>>>
>>> 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm: rcar-du: dw-hdmi: Reject modes with a too high clock frequency
2018-11-23 15:30 ` Kieran Bingham
@ 2018-11-23 16:46 ` Laurent Pinchart
-1 siblings, 0 replies; 10+ messages in thread
From: Laurent Pinchart @ 2018-11-23 16:46 UTC (permalink / raw)
To: kieran.bingham; +Cc: Laurent Pinchart, dri-devel, linux-renesas-soc
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@ideasonboard.com>
> >>> ---
> >>>
> >>> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm: rcar-du: dw-hdmi: Reject modes with a too high clock frequency
@ 2018-11-23 16:46 ` Laurent Pinchart
0 siblings, 0 replies; 10+ messages in thread
From: Laurent Pinchart @ 2018-11-23 16:46 UTC (permalink / raw)
To: kieran.bingham; +Cc: linux-renesas-soc, Laurent Pinchart, dri-devel
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@ideasonboard.com>
> >>> ---
> >>>
> >>> 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-11-24 3:30 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-23 14:34 [PATCH] drm: rcar-du: dw-hdmi: Reject modes with a too high clock frequency Laurent Pinchart
2018-11-23 14:34 ` Laurent Pinchart
2018-11-23 14:43 ` Kieran Bingham
2018-11-23 14:43 ` Kieran Bingham
2018-11-23 14:47 ` Laurent Pinchart
2018-11-23 14:47 ` Laurent Pinchart
2018-11-23 15:30 ` Kieran Bingham
2018-11-23 15:30 ` Kieran Bingham
2018-11-23 16:46 ` Laurent Pinchart
2018-11-23 16:46 ` Laurent Pinchart
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.