diff mbox series

[v2,12/13] drm/bridge: lt9611: stop filtering modes via the table

Message ID 20230108165656.136871-13-dmitry.baryshkov@linaro.org (mailing list archive)
State Superseded
Headers show
Series drm/bridge: lt9611: several fixes and improvements | expand

Commit Message

Dmitry Baryshkov Jan. 8, 2023, 4:56 p.m. UTC 
The lt9611 bridge can support different modes, it makes no sense to list
them in the table. Drop the table and check the number of interfaces
using the fixed value.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/bridge/lontium-lt9611.c | 41 +++----------------------
 1 file changed, 4 insertions(+), 37 deletions(-)

Comments

Neil Armstrong Jan. 11, 2023, 10:57 a.m. UTC | #1 
On 08/01/2023 17:56, Dmitry Baryshkov wrote:
> The lt9611 bridge can support different modes, it makes no sense to list
> them in the table. Drop the table and check the number of interfaces
> using the fixed value.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   drivers/gpu/drm/bridge/lontium-lt9611.c | 41 +++----------------------
>   1 file changed, 4 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c b/drivers/gpu/drm/bridge/lontium-lt9611.c
> index 82af1f954cc6..df9f015aa3a0 100644
> --- a/drivers/gpu/drm/bridge/lontium-lt9611.c
> +++ b/drivers/gpu/drm/bridge/lontium-lt9611.c
> @@ -84,24 +84,6 @@ static const struct regmap_config lt9611_regmap_config = {
>   	.num_ranges = ARRAY_SIZE(lt9611_ranges),
>   };
>   
> -struct lt9611_mode {
> -	u16 hdisplay;
> -	u16 vdisplay;
> -	u8 vrefresh;
> -	u8 lanes;
> -	u8 intfs;
> -};
> -
> -static struct lt9611_mode lt9611_modes[] = {
> -	{ 3840, 2160, 30, 4, 2 }, /* 3840x2160 24bit 30Hz 4Lane 2ports */
> -	{ 1920, 1080, 60, 4, 1 }, /* 1080P 24bit 60Hz 4lane 1port */
> -	{ 1920, 1080, 30, 3, 1 }, /* 1080P 24bit 30Hz 3lane 1port */
> -	{ 1920, 1080, 24, 3, 1 },
> -	{ 720, 480, 60, 4, 1 },
> -	{ 720, 576, 50, 2, 1 },
> -	{ 640, 480, 60, 2, 1 },
> -};
> -
>   static struct lt9611 *bridge_to_lt9611(struct drm_bridge *bridge)
>   {
>   	return container_of(bridge, struct lt9611, bridge);
> @@ -603,21 +585,6 @@ static int lt9611_regulator_enable(struct lt9611 *lt9611)
>   	return 0;
>   }
>   
> -static struct lt9611_mode *lt9611_find_mode(const struct drm_display_mode *mode)
> -{
> -	int i;
> -
> -	for (i = 0; i < ARRAY_SIZE(lt9611_modes); i++) {
> -		if (lt9611_modes[i].hdisplay == mode->hdisplay &&
> -		    lt9611_modes[i].vdisplay == mode->vdisplay &&
> -		    lt9611_modes[i].vrefresh == drm_mode_vrefresh(mode)) {
> -			return &lt9611_modes[i];
> -		}
> -	}
> -
> -	return NULL;
> -}
> -
>   static enum drm_connector_status lt9611_bridge_detect(struct drm_bridge *bridge)
>   {
>   	struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
> @@ -832,12 +799,12 @@ static enum drm_mode_status lt9611_bridge_mode_valid(struct drm_bridge *bridge,
>   						     const struct drm_display_info *info,
>   						     const struct drm_display_mode *mode)
>   {
> -	struct lt9611_mode *lt9611_mode = lt9611_find_mode(mode);
>   	struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
>   
> -	if (!lt9611_mode)
> -		return MODE_BAD;
> -	else if (lt9611_mode->intfs > 1 && !lt9611->dsi1)
> +	if (mode->hdisplay >= 3840 && drm_mode_vrefresh(mode) >= 31)

Isn't 31 a typo ?

> +		return MODE_CLOCK_HIGH;
> +
> +	if (mode->hdisplay > 2000 && !lt9611->dsi1_node)
>   		return MODE_PANEL;
>   	else
>   		return MODE_OK;
Dmitry Baryshkov Jan. 11, 2023, 3:37 p.m. UTC | #2 
On 11/01/2023 12:57, Neil Armstrong wrote:
> On 08/01/2023 17:56, Dmitry Baryshkov wrote:
>> The lt9611 bridge can support different modes, it makes no sense to list
>> them in the table. Drop the table and check the number of interfaces
>> using the fixed value.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>>   drivers/gpu/drm/bridge/lontium-lt9611.c | 41 +++----------------------
>>   1 file changed, 4 insertions(+), 37 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c 
>> b/drivers/gpu/drm/bridge/lontium-lt9611.c
>> index 82af1f954cc6..df9f015aa3a0 100644
>> --- a/drivers/gpu/drm/bridge/lontium-lt9611.c
>> +++ b/drivers/gpu/drm/bridge/lontium-lt9611.c
>> @@ -84,24 +84,6 @@ static const struct regmap_config 
>> lt9611_regmap_config = {
>>       .num_ranges = ARRAY_SIZE(lt9611_ranges),
>>   };
>> -struct lt9611_mode {
>> -    u16 hdisplay;
>> -    u16 vdisplay;
>> -    u8 vrefresh;
>> -    u8 lanes;
>> -    u8 intfs;
>> -};
>> -
>> -static struct lt9611_mode lt9611_modes[] = {
>> -    { 3840, 2160, 30, 4, 2 }, /* 3840x2160 24bit 30Hz 4Lane 2ports */
>> -    { 1920, 1080, 60, 4, 1 }, /* 1080P 24bit 60Hz 4lane 1port */
>> -    { 1920, 1080, 30, 3, 1 }, /* 1080P 24bit 30Hz 3lane 1port */
>> -    { 1920, 1080, 24, 3, 1 },
>> -    { 720, 480, 60, 4, 1 },
>> -    { 720, 576, 50, 2, 1 },
>> -    { 640, 480, 60, 2, 1 },
>> -};
>> -
>>   static struct lt9611 *bridge_to_lt9611(struct drm_bridge *bridge)
>>   {
>>       return container_of(bridge, struct lt9611, bridge);
>> @@ -603,21 +585,6 @@ static int lt9611_regulator_enable(struct lt9611 
>> *lt9611)
>>       return 0;
>>   }
>> -static struct lt9611_mode *lt9611_find_mode(const struct 
>> drm_display_mode *mode)
>> -{
>> -    int i;
>> -
>> -    for (i = 0; i < ARRAY_SIZE(lt9611_modes); i++) {
>> -        if (lt9611_modes[i].hdisplay == mode->hdisplay &&
>> -            lt9611_modes[i].vdisplay == mode->vdisplay &&
>> -            lt9611_modes[i].vrefresh == drm_mode_vrefresh(mode)) {
>> -            return &lt9611_modes[i];
>> -        }
>> -    }
>> -
>> -    return NULL;
>> -}
>> -
>>   static enum drm_connector_status lt9611_bridge_detect(struct 
>> drm_bridge *bridge)
>>   {
>>       struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
>> @@ -832,12 +799,12 @@ static enum drm_mode_status 
>> lt9611_bridge_mode_valid(struct drm_bridge *bridge,
>>                                const struct drm_display_info *info,
>>                                const struct drm_display_mode *mode)
>>   {
>> -    struct lt9611_mode *lt9611_mode = lt9611_find_mode(mode);
>>       struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
>> -    if (!lt9611_mode)
>> -        return MODE_BAD;
>> -    else if (lt9611_mode->intfs > 1 && !lt9611->dsi1)
>> +    if (mode->hdisplay >= 3840 && drm_mode_vrefresh(mode) >= 31)
> 
> Isn't 31 a typo ?

Maybe I should change that to drm_mode_vrefresh(mode) > 30. The chip 
supports 3840x2160-30, but doesn't promise to support anything above that.

> 
>> +        return MODE_CLOCK_HIGH;
>> +
>> +    if (mode->hdisplay > 2000 && !lt9611->dsi1_node)
>>           return MODE_PANEL;
>>       else
>>           return MODE_OK;
>
Neil Armstrong Jan. 12, 2023, 8:43 a.m. UTC | #3 
On 11/01/2023 16:37, Dmitry Baryshkov wrote:
> On 11/01/2023 12:57, Neil Armstrong wrote:
>> On 08/01/2023 17:56, Dmitry Baryshkov wrote:
>>> The lt9611 bridge can support different modes, it makes no sense to list
>>> them in the table. Drop the table and check the number of interfaces
>>> using the fixed value.
>>>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> ---
>>>   drivers/gpu/drm/bridge/lontium-lt9611.c | 41 +++----------------------
>>>   1 file changed, 4 insertions(+), 37 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c b/drivers/gpu/drm/bridge/lontium-lt9611.c
>>> index 82af1f954cc6..df9f015aa3a0 100644
>>> --- a/drivers/gpu/drm/bridge/lontium-lt9611.c
>>> +++ b/drivers/gpu/drm/bridge/lontium-lt9611.c
>>> @@ -84,24 +84,6 @@ static const struct regmap_config lt9611_regmap_config = {
>>>       .num_ranges = ARRAY_SIZE(lt9611_ranges),
>>>   };
>>> -struct lt9611_mode {
>>> -    u16 hdisplay;
>>> -    u16 vdisplay;
>>> -    u8 vrefresh;
>>> -    u8 lanes;
>>> -    u8 intfs;
>>> -};
>>> -
>>> -static struct lt9611_mode lt9611_modes[] = {
>>> -    { 3840, 2160, 30, 4, 2 }, /* 3840x2160 24bit 30Hz 4Lane 2ports */
>>> -    { 1920, 1080, 60, 4, 1 }, /* 1080P 24bit 60Hz 4lane 1port */
>>> -    { 1920, 1080, 30, 3, 1 }, /* 1080P 24bit 30Hz 3lane 1port */
>>> -    { 1920, 1080, 24, 3, 1 },
>>> -    { 720, 480, 60, 4, 1 },
>>> -    { 720, 576, 50, 2, 1 },
>>> -    { 640, 480, 60, 2, 1 },
>>> -};
>>> -
>>>   static struct lt9611 *bridge_to_lt9611(struct drm_bridge *bridge)
>>>   {
>>>       return container_of(bridge, struct lt9611, bridge);
>>> @@ -603,21 +585,6 @@ static int lt9611_regulator_enable(struct lt9611 *lt9611)
>>>       return 0;
>>>   }
>>> -static struct lt9611_mode *lt9611_find_mode(const struct drm_display_mode *mode)
>>> -{
>>> -    int i;
>>> -
>>> -    for (i = 0; i < ARRAY_SIZE(lt9611_modes); i++) {
>>> -        if (lt9611_modes[i].hdisplay == mode->hdisplay &&
>>> -            lt9611_modes[i].vdisplay == mode->vdisplay &&
>>> -            lt9611_modes[i].vrefresh == drm_mode_vrefresh(mode)) {
>>> -            return &lt9611_modes[i];
>>> -        }
>>> -    }
>>> -
>>> -    return NULL;
>>> -}
>>> -
>>>   static enum drm_connector_status lt9611_bridge_detect(struct drm_bridge *bridge)
>>>   {
>>>       struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
>>> @@ -832,12 +799,12 @@ static enum drm_mode_status lt9611_bridge_mode_valid(struct drm_bridge *bridge,
>>>                                const struct drm_display_info *info,
>>>                                const struct drm_display_mode *mode)
>>>   {
>>> -    struct lt9611_mode *lt9611_mode = lt9611_find_mode(mode);
>>>       struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
>>> -    if (!lt9611_mode)
>>> -        return MODE_BAD;
>>> -    else if (lt9611_mode->intfs > 1 && !lt9611->dsi1)
>>> +    if (mode->hdisplay >= 3840 && drm_mode_vrefresh(mode) >= 31)
>>
>> Isn't 31 a typo ?
> 
> Maybe I should change that to drm_mode_vrefresh(mode) > 30. The chip supports 3840x2160-30, but doesn't promise to support anything above that.

Yep >= 31 is valid, but > 30 seems more logical.

Concerning the hdisplay check, shouldn't be separate ?

You should switch to:
if (mode->hdisplay > 3840)
    return MODE_BAD_WIDTH;

if (mode->hdisplay == 3840 && drm_mode_vrefresh(mode) > 30)
    return MODE_CLOCK_HIGH;

Isn't there limits on vdisplay aswell ?

Neil

> 
>>
>>> +        return MODE_CLOCK_HIGH;
>>> +
>>> +    if (mode->hdisplay > 2000 && !lt9611->dsi1_node)
>>>           return MODE_PANEL;
>>>       else
>>>           return MODE_OK;
>>
>
Dmitry Baryshkov Jan. 12, 2023, 9:19 a.m. UTC | #4 
On 12/01/2023 10:43, neil.armstrong@linaro.org wrote:
> On 11/01/2023 16:37, Dmitry Baryshkov wrote:
>> On 11/01/2023 12:57, Neil Armstrong wrote:
>>> On 08/01/2023 17:56, Dmitry Baryshkov wrote:
>>>> The lt9611 bridge can support different modes, it makes no sense to 
>>>> list
>>>> them in the table. Drop the table and check the number of interfaces
>>>> using the fixed value.
>>>>
>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>> ---
>>>>   drivers/gpu/drm/bridge/lontium-lt9611.c | 41 
>>>> +++----------------------
>>>>   1 file changed, 4 insertions(+), 37 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c 
>>>> b/drivers/gpu/drm/bridge/lontium-lt9611.c
>>>> index 82af1f954cc6..df9f015aa3a0 100644
>>>> --- a/drivers/gpu/drm/bridge/lontium-lt9611.c
>>>> +++ b/drivers/gpu/drm/bridge/lontium-lt9611.c
>>>> @@ -84,24 +84,6 @@ static const struct regmap_config 
>>>> lt9611_regmap_config = {
>>>>       .num_ranges = ARRAY_SIZE(lt9611_ranges),
>>>>   };
>>>> -struct lt9611_mode {
>>>> -    u16 hdisplay;
>>>> -    u16 vdisplay;
>>>> -    u8 vrefresh;
>>>> -    u8 lanes;
>>>> -    u8 intfs;
>>>> -};
>>>> -
>>>> -static struct lt9611_mode lt9611_modes[] = {
>>>> -    { 3840, 2160, 30, 4, 2 }, /* 3840x2160 24bit 30Hz 4Lane 2ports */
>>>> -    { 1920, 1080, 60, 4, 1 }, /* 1080P 24bit 60Hz 4lane 1port */
>>>> -    { 1920, 1080, 30, 3, 1 }, /* 1080P 24bit 30Hz 3lane 1port */
>>>> -    { 1920, 1080, 24, 3, 1 },
>>>> -    { 720, 480, 60, 4, 1 },
>>>> -    { 720, 576, 50, 2, 1 },
>>>> -    { 640, 480, 60, 2, 1 },
>>>> -};
>>>> -
>>>>   static struct lt9611 *bridge_to_lt9611(struct drm_bridge *bridge)
>>>>   {
>>>>       return container_of(bridge, struct lt9611, bridge);
>>>> @@ -603,21 +585,6 @@ static int lt9611_regulator_enable(struct 
>>>> lt9611 *lt9611)
>>>>       return 0;
>>>>   }
>>>> -static struct lt9611_mode *lt9611_find_mode(const struct 
>>>> drm_display_mode *mode)
>>>> -{
>>>> -    int i;
>>>> -
>>>> -    for (i = 0; i < ARRAY_SIZE(lt9611_modes); i++) {
>>>> -        if (lt9611_modes[i].hdisplay == mode->hdisplay &&
>>>> -            lt9611_modes[i].vdisplay == mode->vdisplay &&
>>>> -            lt9611_modes[i].vrefresh == drm_mode_vrefresh(mode)) {
>>>> -            return &lt9611_modes[i];
>>>> -        }
>>>> -    }
>>>> -
>>>> -    return NULL;
>>>> -}
>>>> -
>>>>   static enum drm_connector_status lt9611_bridge_detect(struct 
>>>> drm_bridge *bridge)
>>>>   {
>>>>       struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
>>>> @@ -832,12 +799,12 @@ static enum drm_mode_status 
>>>> lt9611_bridge_mode_valid(struct drm_bridge *bridge,
>>>>                                const struct drm_display_info *info,
>>>>                                const struct drm_display_mode *mode)
>>>>   {
>>>> -    struct lt9611_mode *lt9611_mode = lt9611_find_mode(mode);
>>>>       struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
>>>> -    if (!lt9611_mode)
>>>> -        return MODE_BAD;
>>>> -    else if (lt9611_mode->intfs > 1 && !lt9611->dsi1)
>>>> +    if (mode->hdisplay >= 3840 && drm_mode_vrefresh(mode) >= 31)
>>>
>>> Isn't 31 a typo ?
>>
>> Maybe I should change that to drm_mode_vrefresh(mode) > 30. The chip 
>> supports 3840x2160-30, but doesn't promise to support anything above 
>> that.
> 
> Yep >= 31 is valid, but > 30 seems more logical.
> 
> Concerning the hdisplay check, shouldn't be separate ?
> 
> You should switch to:
> if (mode->hdisplay > 3840)
>     return MODE_BAD_WIDTH;
> 
> if (mode->hdisplay == 3840 && drm_mode_vrefresh(mode) > 30)
>     return MODE_CLOCK_HIGH;

Good idea, I'll adapt it for v3.

> 
> Isn't there limits on vdisplay aswell ?

I don't see any special limit in the datasheet. HDMI 1.4, 4k@30, that's 
all. I think I'll just add vdisply > 2160 check next to hdisplay.

> 
> Neil
> 
>>
>>>
>>>> +        return MODE_CLOCK_HIGH;
>>>> +
>>>> +    if (mode->hdisplay > 2000 && !lt9611->dsi1_node)
>>>>           return MODE_PANEL;
>>>>       else
>>>>           return MODE_OK;
>>>
>>
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/bridge/lontium-lt9611.c b/drivers/gpu/drm/bridge/lontium-lt9611.c
index 82af1f954cc6..df9f015aa3a0 100644
--- a/drivers/gpu/drm/bridge/lontium-lt9611.c
+++ b/drivers/gpu/drm/bridge/lontium-lt9611.c
@@ -84,24 +84,6 @@  static const struct regmap_config lt9611_regmap_config = {
 	.num_ranges = ARRAY_SIZE(lt9611_ranges),
 };
 
-struct lt9611_mode {
-	u16 hdisplay;
-	u16 vdisplay;
-	u8 vrefresh;
-	u8 lanes;
-	u8 intfs;
-};
-
-static struct lt9611_mode lt9611_modes[] = {
-	{ 3840, 2160, 30, 4, 2 }, /* 3840x2160 24bit 30Hz 4Lane 2ports */
-	{ 1920, 1080, 60, 4, 1 }, /* 1080P 24bit 60Hz 4lane 1port */
-	{ 1920, 1080, 30, 3, 1 }, /* 1080P 24bit 30Hz 3lane 1port */
-	{ 1920, 1080, 24, 3, 1 },
-	{ 720, 480, 60, 4, 1 },
-	{ 720, 576, 50, 2, 1 },
-	{ 640, 480, 60, 2, 1 },
-};
-
 static struct lt9611 *bridge_to_lt9611(struct drm_bridge *bridge)
 {
 	return container_of(bridge, struct lt9611, bridge);
@@ -603,21 +585,6 @@  static int lt9611_regulator_enable(struct lt9611 *lt9611)
 	return 0;
 }
 
-static struct lt9611_mode *lt9611_find_mode(const struct drm_display_mode *mode)
-{
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(lt9611_modes); i++) {
-		if (lt9611_modes[i].hdisplay == mode->hdisplay &&
-		    lt9611_modes[i].vdisplay == mode->vdisplay &&
-		    lt9611_modes[i].vrefresh == drm_mode_vrefresh(mode)) {
-			return &lt9611_modes[i];
-		}
-	}
-
-	return NULL;
-}
-
 static enum drm_connector_status lt9611_bridge_detect(struct drm_bridge *bridge)
 {
 	struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
@@ -832,12 +799,12 @@  static enum drm_mode_status lt9611_bridge_mode_valid(struct drm_bridge *bridge,
 						     const struct drm_display_info *info,
 						     const struct drm_display_mode *mode)
 {
-	struct lt9611_mode *lt9611_mode = lt9611_find_mode(mode);
 	struct lt9611 *lt9611 = bridge_to_lt9611(bridge);
 
-	if (!lt9611_mode)
-		return MODE_BAD;
-	else if (lt9611_mode->intfs > 1 && !lt9611->dsi1)
+	if (mode->hdisplay >= 3840 && drm_mode_vrefresh(mode) >= 31)
+		return MODE_CLOCK_HIGH;
+
+	if (mode->hdisplay > 2000 && !lt9611->dsi1_node)
 		return MODE_PANEL;
 	else
 		return MODE_OK;