Re: [PATCH v3 35/56] drm/omap: dsi: implement check timings

2020-11-11 Thread Tomi Valkeinen
On 09/11/2020 12:47, Laurent Pinchart wrote:
> Hi Tomi and Sebastian,
> 
> Thank you for the patch.
> 
> On Thu, Nov 05, 2020 at 02:03:12PM +0200, Tomi Valkeinen wrote:
>> From: Sebastian Reichel 
>>
>> Implement check timings, which will check if its possible to
> 
> s/its/it's/
> 
>> configure the clocks for the provided mode using the same code
>> as the set_config() hook.
>>
>> Signed-off-by: Sebastian Reichel 
>> Signed-off-by: Tomi Valkeinen 
>> ---
>>  drivers/gpu/drm/omapdrm/dss/dsi.c | 70 +++
>>  1 file changed, 44 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c 
>> b/drivers/gpu/drm/omapdrm/dss/dsi.c
>> index a1a867a7d91d..f643321434e9 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/dsi.c
>> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
>> @@ -280,6 +280,11 @@ struct dsi_isr_tables {
>>  struct dsi_isr_data isr_table_cio[DSI_MAX_NR_ISRS];
>>  };
>>  
>> +struct dsi_lp_clock_info {
>> +unsigned long lp_clk;
>> +u16 lp_clk_div;
>> +};
>> +
>>  struct dsi_clk_calc_ctx {
>>  struct dsi_data *dsi;
>>  struct dss_pll *pll;
>> @@ -294,16 +299,12 @@ struct dsi_clk_calc_ctx {
>>  
>>  struct dss_pll_clock_info dsi_cinfo;
>>  struct dispc_clock_info dispc_cinfo;
>> +struct dsi_lp_clock_info user_lp_cinfo;
> 
> Any reason for the user_ prefix here ?

No, I'll drop it.

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3 35/56] drm/omap: dsi: implement check timings

2020-11-09 Thread Laurent Pinchart
Hi Tomi and Sebastian,

Thank you for the patch.

On Thu, Nov 05, 2020 at 02:03:12PM +0200, Tomi Valkeinen wrote:
> From: Sebastian Reichel 
> 
> Implement check timings, which will check if its possible to

s/its/it's/

> configure the clocks for the provided mode using the same code
> as the set_config() hook.
> 
> Signed-off-by: Sebastian Reichel 
> Signed-off-by: Tomi Valkeinen 
> ---
>  drivers/gpu/drm/omapdrm/dss/dsi.c | 70 +++
>  1 file changed, 44 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c 
> b/drivers/gpu/drm/omapdrm/dss/dsi.c
> index a1a867a7d91d..f643321434e9 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dsi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
> @@ -280,6 +280,11 @@ struct dsi_isr_tables {
>   struct dsi_isr_data isr_table_cio[DSI_MAX_NR_ISRS];
>  };
>  
> +struct dsi_lp_clock_info {
> + unsigned long lp_clk;
> + u16 lp_clk_div;
> +};
> +
>  struct dsi_clk_calc_ctx {
>   struct dsi_data *dsi;
>   struct dss_pll *pll;
> @@ -294,16 +299,12 @@ struct dsi_clk_calc_ctx {
>  
>   struct dss_pll_clock_info dsi_cinfo;
>   struct dispc_clock_info dispc_cinfo;
> + struct dsi_lp_clock_info user_lp_cinfo;

Any reason for the user_ prefix here ?

Reviewed-by: Laurent Pinchart 

>  
>   struct videomode vm;
>   struct omap_dss_dsi_videomode_timings dsi_vm;
>  };
>  
> -struct dsi_lp_clock_info {
> - unsigned long lp_clk;
> - u16 lp_clk_div;
> -};
> -
>  struct dsi_module_id_data {
>   u32 address;
>   int id;
> @@ -4789,44 +4790,55 @@ static bool dsi_is_video_mode(struct omap_dss_device 
> *dssdev)
>   return (dsi->mode == OMAP_DSS_DSI_VIDEO_MODE);
>  }
>  
> -static int dsi_set_config(struct omap_dss_device *dssdev,
> - const struct drm_display_mode *mode)
> +static int __dsi_calc_config(struct dsi_data *dsi,
> + const struct drm_display_mode *mode,
> + struct dsi_clk_calc_ctx *ctx)
>  {
> - struct dsi_data *dsi = to_dsi_data(dssdev);
> - struct dsi_clk_calc_ctx ctx;
> - struct videomode vm;
>   struct omap_dss_dsi_config cfg = dsi->config;
> + struct videomode vm;
>   bool ok;
>   int r;
>  
>   drm_display_mode_to_videomode(mode, );
> - cfg.vm = 
> -
> - mutex_lock(>lock);
>  
> + cfg.vm = 
>   cfg.mode = dsi->mode;
>   cfg.pixel_format = dsi->pix_fmt;
>  
>   if (dsi->mode == OMAP_DSS_DSI_VIDEO_MODE)
> - ok = dsi_vm_calc(dsi, , );
> + ok = dsi_vm_calc(dsi, , ctx);
>   else
> - ok = dsi_cm_calc(dsi, , );
> + ok = dsi_cm_calc(dsi, , ctx);
>  
> - if (!ok) {
> - DSSERR("failed to find suitable DSI clock settings\n");
> - r = -EINVAL;
> - goto err;
> - }
> + if (!ok)
> + return -EINVAL;
> +
> + dsi_pll_calc_dsi_fck(dsi, >dsi_cinfo);
>  
> - dsi_pll_calc_dsi_fck(dsi, _cinfo);
> + r = dsi_lp_clock_calc(ctx->dsi_cinfo.clkout[HSDIV_DSI],
> + cfg.lp_clk_min, cfg.lp_clk_max, >user_lp_cinfo);
> + if (r)
> + return r;
> +
> + return 0;
> +}
>  
> - r = dsi_lp_clock_calc(ctx.dsi_cinfo.clkout[HSDIV_DSI],
> - cfg.lp_clk_min, cfg.lp_clk_max, >user_lp_cinfo);
> +static int dsi_set_config(struct omap_dss_device *dssdev,
> + const struct drm_display_mode *mode)
> +{
> + struct dsi_data *dsi = to_dsi_data(dssdev);
> + struct dsi_clk_calc_ctx ctx;
> + int r;
> +
> + mutex_lock(>lock);
> +
> + r = __dsi_calc_config(dsi, mode, );
>   if (r) {
> - DSSERR("failed to find suitable DSI LP clock settings\n");
> + DSSERR("failed to find suitable DSI clock settings\n");
>   goto err;
>   }
>  
> + dsi->user_lp_cinfo = ctx.user_lp_cinfo;
>   dsi->user_dsi_cinfo = ctx.dsi_cinfo;
>   dsi->user_dispc_cinfo = ctx.dispc_cinfo;
>  
> @@ -5004,11 +5016,17 @@ static void dsi_set_timings(struct omap_dss_device 
> *dssdev,
>  static int dsi_check_timings(struct omap_dss_device *dssdev,
>struct drm_display_mode *mode)
>  {
> + struct dsi_data *dsi = to_dsi_data(dssdev);
> + struct dsi_clk_calc_ctx ctx;
> + int r;
> +
>   DSSDBG("dsi_check_timings\n");
>  
> - /* TODO */
> + mutex_lock(>lock);
> + r = __dsi_calc_config(dsi, mode, );
> + mutex_unlock(>lock);
>  
> - return 0;
> + return r;
>  }
>  
>  static int dsi_connect(struct omap_dss_device *src,

-- 
Regards,

Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v3 35/56] drm/omap: dsi: implement check timings

2020-11-05 Thread Tomi Valkeinen
From: Sebastian Reichel 

Implement check timings, which will check if its possible to
configure the clocks for the provided mode using the same code
as the set_config() hook.

Signed-off-by: Sebastian Reichel 
Signed-off-by: Tomi Valkeinen 
---
 drivers/gpu/drm/omapdrm/dss/dsi.c | 70 +++
 1 file changed, 44 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c 
b/drivers/gpu/drm/omapdrm/dss/dsi.c
index a1a867a7d91d..f643321434e9 100644
--- a/drivers/gpu/drm/omapdrm/dss/dsi.c
+++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
@@ -280,6 +280,11 @@ struct dsi_isr_tables {
struct dsi_isr_data isr_table_cio[DSI_MAX_NR_ISRS];
 };
 
+struct dsi_lp_clock_info {
+   unsigned long lp_clk;
+   u16 lp_clk_div;
+};
+
 struct dsi_clk_calc_ctx {
struct dsi_data *dsi;
struct dss_pll *pll;
@@ -294,16 +299,12 @@ struct dsi_clk_calc_ctx {
 
struct dss_pll_clock_info dsi_cinfo;
struct dispc_clock_info dispc_cinfo;
+   struct dsi_lp_clock_info user_lp_cinfo;
 
struct videomode vm;
struct omap_dss_dsi_videomode_timings dsi_vm;
 };
 
-struct dsi_lp_clock_info {
-   unsigned long lp_clk;
-   u16 lp_clk_div;
-};
-
 struct dsi_module_id_data {
u32 address;
int id;
@@ -4789,44 +4790,55 @@ static bool dsi_is_video_mode(struct omap_dss_device 
*dssdev)
return (dsi->mode == OMAP_DSS_DSI_VIDEO_MODE);
 }
 
-static int dsi_set_config(struct omap_dss_device *dssdev,
-   const struct drm_display_mode *mode)
+static int __dsi_calc_config(struct dsi_data *dsi,
+   const struct drm_display_mode *mode,
+   struct dsi_clk_calc_ctx *ctx)
 {
-   struct dsi_data *dsi = to_dsi_data(dssdev);
-   struct dsi_clk_calc_ctx ctx;
-   struct videomode vm;
struct omap_dss_dsi_config cfg = dsi->config;
+   struct videomode vm;
bool ok;
int r;
 
drm_display_mode_to_videomode(mode, );
-   cfg.vm = 
-
-   mutex_lock(>lock);
 
+   cfg.vm = 
cfg.mode = dsi->mode;
cfg.pixel_format = dsi->pix_fmt;
 
if (dsi->mode == OMAP_DSS_DSI_VIDEO_MODE)
-   ok = dsi_vm_calc(dsi, , );
+   ok = dsi_vm_calc(dsi, , ctx);
else
-   ok = dsi_cm_calc(dsi, , );
+   ok = dsi_cm_calc(dsi, , ctx);
 
-   if (!ok) {
-   DSSERR("failed to find suitable DSI clock settings\n");
-   r = -EINVAL;
-   goto err;
-   }
+   if (!ok)
+   return -EINVAL;
+
+   dsi_pll_calc_dsi_fck(dsi, >dsi_cinfo);
 
-   dsi_pll_calc_dsi_fck(dsi, _cinfo);
+   r = dsi_lp_clock_calc(ctx->dsi_cinfo.clkout[HSDIV_DSI],
+   cfg.lp_clk_min, cfg.lp_clk_max, >user_lp_cinfo);
+   if (r)
+   return r;
+
+   return 0;
+}
 
-   r = dsi_lp_clock_calc(ctx.dsi_cinfo.clkout[HSDIV_DSI],
-   cfg.lp_clk_min, cfg.lp_clk_max, >user_lp_cinfo);
+static int dsi_set_config(struct omap_dss_device *dssdev,
+   const struct drm_display_mode *mode)
+{
+   struct dsi_data *dsi = to_dsi_data(dssdev);
+   struct dsi_clk_calc_ctx ctx;
+   int r;
+
+   mutex_lock(>lock);
+
+   r = __dsi_calc_config(dsi, mode, );
if (r) {
-   DSSERR("failed to find suitable DSI LP clock settings\n");
+   DSSERR("failed to find suitable DSI clock settings\n");
goto err;
}
 
+   dsi->user_lp_cinfo = ctx.user_lp_cinfo;
dsi->user_dsi_cinfo = ctx.dsi_cinfo;
dsi->user_dispc_cinfo = ctx.dispc_cinfo;
 
@@ -5004,11 +5016,17 @@ static void dsi_set_timings(struct omap_dss_device 
*dssdev,
 static int dsi_check_timings(struct omap_dss_device *dssdev,
 struct drm_display_mode *mode)
 {
+   struct dsi_data *dsi = to_dsi_data(dssdev);
+   struct dsi_clk_calc_ctx ctx;
+   int r;
+
DSSDBG("dsi_check_timings\n");
 
-   /* TODO */
+   mutex_lock(>lock);
+   r = __dsi_calc_config(dsi, mode, );
+   mutex_unlock(>lock);
 
-   return 0;
+   return r;
 }
 
 static int dsi_connect(struct omap_dss_device *src,
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel