Add the atomic_check hook to ensure that the parameters are within the
valid range.
As of now, dsi clock freqency is being calculated in bridge_enable but
this needs to be checked in atomic_check which is called before
bridge_enable so move this calculation to atomic_check and write the
register value in bridge_enable as it is.
For now, add mode clock check for the max resolution supported by the
bridge as mentioned in the SN65DSI86 datasheet[0] and dsi clock range
check for SN_DSIA_CLK_FREQ_REG.
According to the datasheet[0], the minimum value for that reg is 0x08
and the maximum value is 0x96. So add check for that.
[0]: <https://www.ti.com/lit/gpn/sn65dsi86>
Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
---
drivers/gpu/drm/bridge/ti-sn65dsi86.c | 65 +++++++++++++++++++--------
1 file changed, 46 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 84698a0b27a8..d13b42d7c512 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -113,6 +113,20 @@
#define MIN_DSI_CLK_FREQ_MHZ 40
+/*
+ * NOTE: DSI clock frequency range: [40MHz,755MHz)
+ * DSI clock frequency range is in 5-MHz increments
+ * So [40MHz,45MHz) translates to 0x08 (min value)
+ * And [750MHz,755MHz) translates to 0x96 (max value)
+ */
+#define MIN_DSI_CLK_RANGE 0x8
+#define MAX_DSI_CLK_RANGE 0x96
+
+/* Pixel clock to support max resolution (4K@60Hz) supported
+ * by the bridge.
+ */
+#define SN65DSI86_MAX_PIXEL_CLOCK_KHZ 600000
+
/* fudge factor required to account for 8b/10b encoding */
#define DP_CLK_FUDGE_NUM 10
#define DP_CLK_FUDGE_DEN 8
@@ -191,6 +205,7 @@ struct ti_sn65dsi86 {
u8 ln_polrs;
bool comms_enabled;
struct mutex comms_mutex;
+ u32 dsi_clk_range;
#if defined(CONFIG_OF_GPIO)
struct gpio_chip gchip;
@@ -820,24 +835,6 @@ static void ti_sn_bridge_atomic_disable(struct drm_bridge *bridge,
regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG, VSTREAM_ENABLE, 0);
}
-static void ti_sn_bridge_set_dsi_rate(struct ti_sn65dsi86 *pdata)
-{
- unsigned int bit_rate_mhz, clk_freq_mhz;
- unsigned int val;
- struct drm_display_mode *mode =
- &pdata->bridge.encoder->crtc->state->adjusted_mode;
-
- /* set DSIA clk frequency */
- bit_rate_mhz = (mode->clock / 1000) *
- mipi_dsi_pixel_format_to_bpp(pdata->dsi->format);
- clk_freq_mhz = bit_rate_mhz / (pdata->dsi->lanes * 2);
-
- /* for each increment in val, frequency increases by 5MHz */
- val = (MIN_DSI_CLK_FREQ_MHZ / 5) +
- (((clk_freq_mhz - MIN_DSI_CLK_FREQ_MHZ) / 5) & 0xFF);
- regmap_write(pdata->regmap, SN_DSIA_CLK_FREQ_REG, val);
-}
-
static unsigned int ti_sn_bridge_get_bpp(struct drm_connector *connector)
{
if (connector->display_info.bpc <= 6)
@@ -1104,7 +1101,7 @@ static void ti_sn_bridge_atomic_enable(struct drm_bridge *bridge,
pdata->ln_polrs << LN_POLRS_OFFSET);
/* set dsi clk frequency value */
- ti_sn_bridge_set_dsi_rate(pdata);
+ regmap_write(pdata->regmap, SN_DSIA_CLK_FREQ_REG, pdata->dsi_clk_range);
/*
* The SN65DSI86 only supports ASSR Display Authentication method and
@@ -1215,6 +1212,35 @@ static const struct drm_edid *ti_sn_bridge_edid_read(struct drm_bridge *bridge,
return drm_edid_read_ddc(connector, &pdata->aux.ddc);
}
+static int ti_sn_bridge_atomic_check(struct drm_bridge *bridge,
+ struct drm_bridge_state *bridge_state,
+ struct drm_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state)
+{
+ struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge);
+ struct drm_display_mode *mode = &crtc_state->mode;
+ unsigned int bit_rate_mhz, clk_freq_mhz;
+
+ /* Pixel clock check */
+ if (mode->clock > SN65DSI86_MAX_PIXEL_CLOCK_KHZ)
+ return -EINVAL;
+
+ bit_rate_mhz = (mode->clock / 1000) *
+ mipi_dsi_pixel_format_to_bpp(pdata->dsi->format);
+ clk_freq_mhz = bit_rate_mhz / (pdata->dsi->lanes * 2);
+
+ /* for each increment in dsi_clk_range, frequency increases by 5MHz */
+ pdata->dsi_clk_range = (MIN_DSI_CLK_FREQ_MHZ / 5) +
+ (((clk_freq_mhz - MIN_DSI_CLK_FREQ_MHZ) / 5) & 0xFF);
+
+ /* SN_DSIA_CLK_FREQ_REG check */
+ if (pdata->dsi_clk_range > MAX_DSI_CLK_RANGE ||
+ pdata->dsi_clk_range < MIN_DSI_CLK_RANGE)
+ return -EINVAL;
+
+ return 0;
+}
+
static const struct drm_bridge_funcs ti_sn_bridge_funcs = {
.attach = ti_sn_bridge_attach,
.detach = ti_sn_bridge_detach,
@@ -1228,6 +1254,7 @@ static const struct drm_bridge_funcs ti_sn_bridge_funcs = {
.atomic_reset = drm_atomic_helper_bridge_reset,
.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+ .atomic_check = ti_sn_bridge_atomic_check,
};
static void ti_sn_bridge_parse_lanes(struct ti_sn65dsi86 *pdata,
--
2.25.1On Tue, Jun 18, 2024 at 01:44:17PM GMT, Jayesh Choudhary wrote:
> Add the atomic_check hook to ensure that the parameters are within the
> valid range.
> As of now, dsi clock freqency is being calculated in bridge_enable but
> this needs to be checked in atomic_check which is called before
> bridge_enable so move this calculation to atomic_check and write the
> register value in bridge_enable as it is.
>
> For now, add mode clock check for the max resolution supported by the
> bridge as mentioned in the SN65DSI86 datasheet[0] and dsi clock range
> check for SN_DSIA_CLK_FREQ_REG.
> According to the datasheet[0], the minimum value for that reg is 0x08
> and the maximum value is 0x96. So add check for that.
>
> [0]: <https://www.ti.com/lit/gpn/sn65dsi86>
>
> Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
> ---
> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 65 +++++++++++++++++++--------
> 1 file changed, 46 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index 84698a0b27a8..d13b42d7c512 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -113,6 +113,20 @@
>
> #define MIN_DSI_CLK_FREQ_MHZ 40
>
> +/*
> + * NOTE: DSI clock frequency range: [40MHz,755MHz)
> + * DSI clock frequency range is in 5-MHz increments
> + * So [40MHz,45MHz) translates to 0x08 (min value)
> + * And [750MHz,755MHz) translates to 0x96 (max value)
> + */
> +#define MIN_DSI_CLK_RANGE 0x8
> +#define MAX_DSI_CLK_RANGE 0x96
> +
> +/* Pixel clock to support max resolution (4K@60Hz) supported
> + * by the bridge.
> + */
> +#define SN65DSI86_MAX_PIXEL_CLOCK_KHZ 600000
> +
> /* fudge factor required to account for 8b/10b encoding */
> #define DP_CLK_FUDGE_NUM 10
> #define DP_CLK_FUDGE_DEN 8
> @@ -191,6 +205,7 @@ struct ti_sn65dsi86 {
> u8 ln_polrs;
> bool comms_enabled;
> struct mutex comms_mutex;
> + u32 dsi_clk_range;
>
> #if defined(CONFIG_OF_GPIO)
> struct gpio_chip gchip;
> @@ -820,24 +835,6 @@ static void ti_sn_bridge_atomic_disable(struct drm_bridge *bridge,
> regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG, VSTREAM_ENABLE, 0);
> }
>
> -static void ti_sn_bridge_set_dsi_rate(struct ti_sn65dsi86 *pdata)
> -{
> - unsigned int bit_rate_mhz, clk_freq_mhz;
> - unsigned int val;
> - struct drm_display_mode *mode =
> - &pdata->bridge.encoder->crtc->state->adjusted_mode;
> -
> - /* set DSIA clk frequency */
> - bit_rate_mhz = (mode->clock / 1000) *
> - mipi_dsi_pixel_format_to_bpp(pdata->dsi->format);
> - clk_freq_mhz = bit_rate_mhz / (pdata->dsi->lanes * 2);
> -
> - /* for each increment in val, frequency increases by 5MHz */
> - val = (MIN_DSI_CLK_FREQ_MHZ / 5) +
> - (((clk_freq_mhz - MIN_DSI_CLK_FREQ_MHZ) / 5) & 0xFF);
> - regmap_write(pdata->regmap, SN_DSIA_CLK_FREQ_REG, val);
> -}
> -
> static unsigned int ti_sn_bridge_get_bpp(struct drm_connector *connector)
> {
> if (connector->display_info.bpc <= 6)
> @@ -1104,7 +1101,7 @@ static void ti_sn_bridge_atomic_enable(struct drm_bridge *bridge,
> pdata->ln_polrs << LN_POLRS_OFFSET);
>
> /* set dsi clk frequency value */
> - ti_sn_bridge_set_dsi_rate(pdata);
> + regmap_write(pdata->regmap, SN_DSIA_CLK_FREQ_REG, pdata->dsi_clk_range);
>
> /*
> * The SN65DSI86 only supports ASSR Display Authentication method and
> @@ -1215,6 +1212,35 @@ static const struct drm_edid *ti_sn_bridge_edid_read(struct drm_bridge *bridge,
> return drm_edid_read_ddc(connector, &pdata->aux.ddc);
> }
>
> +static int ti_sn_bridge_atomic_check(struct drm_bridge *bridge,
> + struct drm_bridge_state *bridge_state,
> + struct drm_crtc_state *crtc_state,
> + struct drm_connector_state *conn_state)
> +{
> + struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge);
> + struct drm_display_mode *mode = &crtc_state->mode;
> + unsigned int bit_rate_mhz, clk_freq_mhz;
> +
> + /* Pixel clock check */
> + if (mode->clock > SN65DSI86_MAX_PIXEL_CLOCK_KHZ)
> + return -EINVAL;
> +
> + bit_rate_mhz = (mode->clock / 1000) *
> + mipi_dsi_pixel_format_to_bpp(pdata->dsi->format);
> + clk_freq_mhz = bit_rate_mhz / (pdata->dsi->lanes * 2);
> +
> + /* for each increment in dsi_clk_range, frequency increases by 5MHz */
> + pdata->dsi_clk_range = (MIN_DSI_CLK_FREQ_MHZ / 5) +
> + (((clk_freq_mhz - MIN_DSI_CLK_FREQ_MHZ) / 5) & 0xFF);
atomic_check might be called several times, it might be called to test
the state. As such, it should not modify anything outside of the
state variables.
> +
> + /* SN_DSIA_CLK_FREQ_REG check */
> + if (pdata->dsi_clk_range > MAX_DSI_CLK_RANGE ||
> + pdata->dsi_clk_range < MIN_DSI_CLK_RANGE)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> static const struct drm_bridge_funcs ti_sn_bridge_funcs = {
> .attach = ti_sn_bridge_attach,
> .detach = ti_sn_bridge_detach,
> @@ -1228,6 +1254,7 @@ static const struct drm_bridge_funcs ti_sn_bridge_funcs = {
> .atomic_reset = drm_atomic_helper_bridge_reset,
> .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> + .atomic_check = ti_sn_bridge_atomic_check,
> };
>
> static void ti_sn_bridge_parse_lanes(struct ti_sn65dsi86 *pdata,
> --
> 2.25.1
>
--
With best wishes
Dmitry
Hello Dmitry,
Thanks for the review.
On 18/06/24 14:29, Dmitry Baryshkov wrote:
> On Tue, Jun 18, 2024 at 01:44:17PM GMT, Jayesh Choudhary wrote:
>> Add the atomic_check hook to ensure that the parameters are within the
>> valid range.
>> As of now, dsi clock freqency is being calculated in bridge_enable but
>> this needs to be checked in atomic_check which is called before
>> bridge_enable so move this calculation to atomic_check and write the
>> register value in bridge_enable as it is.
>>
>> For now, add mode clock check for the max resolution supported by the
>> bridge as mentioned in the SN65DSI86 datasheet[0] and dsi clock range
>> check for SN_DSIA_CLK_FREQ_REG.
>> According to the datasheet[0], the minimum value for that reg is 0x08
>> and the maximum value is 0x96. So add check for that.
>>
>> [0]: <https://www.ti.com/lit/gpn/sn65dsi86>
>>
>> Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
>> ---
>> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 65 +++++++++++++++++++--------
>> 1 file changed, 46 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>> index 84698a0b27a8..d13b42d7c512 100644
>> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>> @@ -113,6 +113,20 @@
>>
[...]
>>
>> +static int ti_sn_bridge_atomic_check(struct drm_bridge *bridge,
>> + struct drm_bridge_state *bridge_state,
>> + struct drm_crtc_state *crtc_state,
>> + struct drm_connector_state *conn_state)
>> +{
>> + struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge);
>> + struct drm_display_mode *mode = &crtc_state->mode;
>> + unsigned int bit_rate_mhz, clk_freq_mhz;
>> +
>> + /* Pixel clock check */
>> + if (mode->clock > SN65DSI86_MAX_PIXEL_CLOCK_KHZ)
>> + return -EINVAL;
>> +
>> + bit_rate_mhz = (mode->clock / 1000) *
>> + mipi_dsi_pixel_format_to_bpp(pdata->dsi->format);
>> + clk_freq_mhz = bit_rate_mhz / (pdata->dsi->lanes * 2);
>> +
>> + /* for each increment in dsi_clk_range, frequency increases by 5MHz */
>> + pdata->dsi_clk_range = (MIN_DSI_CLK_FREQ_MHZ / 5) +
>> + (((clk_freq_mhz - MIN_DSI_CLK_FREQ_MHZ) / 5) & 0xFF);
>
> atomic_check might be called several times, it might be called to test
> the state. As such, it should not modify anything outside of the
> state variables.
>
If not in atomic_check, then where should I move this calculation and check?
mode_valid with returning MODE_BAD in case of failure?
I had to move it from bridge_enable based on the comments on v1:
https://patchwork.kernel.org/project/dri-devel/patch/20240408073623.186489-1-j-choudhary@ti.com/#25801801
Warm Regards,
Jayesh
[...]
On Tue, 18 Jun 2024 at 12:56, Jayesh Choudhary <j-choudhary@ti.com> wrote:
>
> Hello Dmitry,
>
> Thanks for the review.
>
> On 18/06/24 14:29, Dmitry Baryshkov wrote:
> > On Tue, Jun 18, 2024 at 01:44:17PM GMT, Jayesh Choudhary wrote:
> >> Add the atomic_check hook to ensure that the parameters are within the
> >> valid range.
> >> As of now, dsi clock freqency is being calculated in bridge_enable but
> >> this needs to be checked in atomic_check which is called before
> >> bridge_enable so move this calculation to atomic_check and write the
> >> register value in bridge_enable as it is.
> >>
> >> For now, add mode clock check for the max resolution supported by the
> >> bridge as mentioned in the SN65DSI86 datasheet[0] and dsi clock range
> >> check for SN_DSIA_CLK_FREQ_REG.
> >> According to the datasheet[0], the minimum value for that reg is 0x08
> >> and the maximum value is 0x96. So add check for that.
> >>
> >> [0]: <https://www.ti.com/lit/gpn/sn65dsi86>
> >>
> >> Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
> >> ---
> >> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 65 +++++++++++++++++++--------
> >> 1 file changed, 46 insertions(+), 19 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> >> index 84698a0b27a8..d13b42d7c512 100644
> >> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> >> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> >> @@ -113,6 +113,20 @@
> >>
>
> [...]
>
> >>
> >> +static int ti_sn_bridge_atomic_check(struct drm_bridge *bridge,
> >> + struct drm_bridge_state *bridge_state,
> >> + struct drm_crtc_state *crtc_state,
> >> + struct drm_connector_state *conn_state)
> >> +{
> >> + struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge);
> >> + struct drm_display_mode *mode = &crtc_state->mode;
> >> + unsigned int bit_rate_mhz, clk_freq_mhz;
> >> +
> >> + /* Pixel clock check */
> >> + if (mode->clock > SN65DSI86_MAX_PIXEL_CLOCK_KHZ)
> >> + return -EINVAL;
> >> +
> >> + bit_rate_mhz = (mode->clock / 1000) *
> >> + mipi_dsi_pixel_format_to_bpp(pdata->dsi->format);
> >> + clk_freq_mhz = bit_rate_mhz / (pdata->dsi->lanes * 2);
> >> +
> >> + /* for each increment in dsi_clk_range, frequency increases by 5MHz */
> >> + pdata->dsi_clk_range = (MIN_DSI_CLK_FREQ_MHZ / 5) +
> >> + (((clk_freq_mhz - MIN_DSI_CLK_FREQ_MHZ) / 5) & 0xFF);
> >
> > atomic_check might be called several times, it might be called to test
> > the state. As such, it should not modify anything outside of the
> > state variables.
> >
>
> If not in atomic_check, then where should I move this calculation and check?
> mode_valid with returning MODE_BAD in case of failure?
I didn't write that it's the wrong place for math. I wrote that you
should not be modifying global structure.
So you have to subclass drm_bridge_state for the driver and store the
value there. Or just add a helper function and call it from
atomic_check(), mode_valid() and set_dsi_rate(). It really looks like
a simpler solution here.
Note, there is a significant difference between mode_valid() and
atomic_check(). The former function is used for filtering the modes,
while the latter one is used for actually checking that the parameters
passed from the client are correct.
>
> I had to move it from bridge_enable based on the comments on v1:
> https://patchwork.kernel.org/project/dri-devel/patch/20240408073623.186489-1-j-choudhary@ti.com/#25801801
>
> Warm Regards,
> Jayesh
>
> [...]
--
With best wishes
Dmitry
Hello Dmitry,
On 18/06/24 15:45, Dmitry Baryshkov wrote:
> On Tue, 18 Jun 2024 at 12:56, Jayesh Choudhary <j-choudhary@ti.com> wrote:
>>
>> Hello Dmitry,
>>
>> Thanks for the review.
>>
>> On 18/06/24 14:29, Dmitry Baryshkov wrote:
>>> On Tue, Jun 18, 2024 at 01:44:17PM GMT, Jayesh Choudhary wrote:
>>>> Add the atomic_check hook to ensure that the parameters are within the
>>>> valid range.
>>>> As of now, dsi clock freqency is being calculated in bridge_enable but
>>>> this needs to be checked in atomic_check which is called before
>>>> bridge_enable so move this calculation to atomic_check and write the
>>>> register value in bridge_enable as it is.
>>>>
>>>> For now, add mode clock check for the max resolution supported by the
>>>> bridge as mentioned in the SN65DSI86 datasheet[0] and dsi clock range
>>>> check for SN_DSIA_CLK_FREQ_REG.
>>>> According to the datasheet[0], the minimum value for that reg is 0x08
>>>> and the maximum value is 0x96. So add check for that.
>>>>
>>>> [0]: <https://www.ti.com/lit/gpn/sn65dsi86>
>>>>
>>>> Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
>>>> ---
>>>> drivers/gpu/drm/bridge/ti-sn65dsi86.c | 65 +++++++++++++++++++--------
>>>> 1 file changed, 46 insertions(+), 19 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>>>> index 84698a0b27a8..d13b42d7c512 100644
>>>> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>>>> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
>>>> @@ -113,6 +113,20 @@
>>>>
>>
>> [...]
>>
>>>>
>>>> +static int ti_sn_bridge_atomic_check(struct drm_bridge *bridge,
>>>> + struct drm_bridge_state *bridge_state,
>>>> + struct drm_crtc_state *crtc_state,
>>>> + struct drm_connector_state *conn_state)
>>>> +{
>>>> + struct ti_sn65dsi86 *pdata = bridge_to_ti_sn65dsi86(bridge);
>>>> + struct drm_display_mode *mode = &crtc_state->mode;
>>>> + unsigned int bit_rate_mhz, clk_freq_mhz;
>>>> +
>>>> + /* Pixel clock check */
>>>> + if (mode->clock > SN65DSI86_MAX_PIXEL_CLOCK_KHZ)
>>>> + return -EINVAL;
>>>> +
>>>> + bit_rate_mhz = (mode->clock / 1000) *
>>>> + mipi_dsi_pixel_format_to_bpp(pdata->dsi->format);
>>>> + clk_freq_mhz = bit_rate_mhz / (pdata->dsi->lanes * 2);
>>>> +
>>>> + /* for each increment in dsi_clk_range, frequency increases by 5MHz */
>>>> + pdata->dsi_clk_range = (MIN_DSI_CLK_FREQ_MHZ / 5) +
>>>> + (((clk_freq_mhz - MIN_DSI_CLK_FREQ_MHZ) / 5) & 0xFF);
>>>
>>> atomic_check might be called several times, it might be called to test
>>> the state. As such, it should not modify anything outside of the
>>> state variables.
>>>
>>
>> If not in atomic_check, then where should I move this calculation and check?
>> mode_valid with returning MODE_BAD in case of failure?
>
> I didn't write that it's the wrong place for math. I wrote that you
> should not be modifying global structure.
>
> So you have to subclass drm_bridge_state for the driver and store the
> value there. Or just add a helper function and call it from
> atomic_check(), mode_valid() and set_dsi_rate(). It really looks like
> a simpler solution here.
>
Okay, instead of moving the set_dsi_rate, I will rename it to
calc_dsi_rate with integer return value which I would use in both
bridge enable to write the register value and atomic_check to check the
parameters eliminating the need to modify the pdata structure/ adding
new variable to the structure.
(Earlier I was trying to avoid calculation in both calls so I added
another variable to the structure and used that. But I get your point
now!)
I will re-order the patches to have an independent fix patch
addressing your concern in [2/2] patch
https://lore.kernel.org/all/CAA8EJpq2UkMn9ArSNaJcOyw28H4uUcRwvUqfUBBqSCALmozBrg@mail.gmail.com/
Also in the code they have been using 594MHz as mode clock limit.
I was using more relaxed value (600MHz) in atomic check but I will
switch to 594MHz to be in sync with the value that is used in the
driver.
> Note, there is a significant difference between mode_valid() and
> atomic_check(). The former function is used for filtering the modes,
> while the latter one is used for actually checking that the parameters
> passed from the client are correct.
[...]
Warm Regards,
Jayesh
© 2016 - 2025 Red Hat, Inc.