2017-03-30 16:19 GMT+09:00 walter harms <wharms@xxxxxx>:
>
>
> Am 30.03.2017 08:25, schrieb Daeseok Youn:
>> Define new local variable to reduce the number of reference.
>> The new local variable is added to save the addess of dfs
>> and used in atomisp_freq_scaling() function.
>>
>> Signed-off-by: Daeseok Youn <daeseok.youn@xxxxxxxxx>
>> ---
>> .../media/atomisp/pci/atomisp2/atomisp_cmd.c | 37 ++++++++++++----------
>> 1 file changed, 20 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
>> index eebfccd..d76a95c 100644
>> --- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
>> +++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
>> @@ -251,6 +251,7 @@ int atomisp_freq_scaling(struct atomisp_device *isp,
>> {
>> /* FIXME! Only use subdev[0] status yet */
>> struct atomisp_sub_device *asd = &isp->asd[0];
>> + const struct atomisp_dfs_config *dfs;
>> unsigned int new_freq;
>> struct atomisp_freq_scaling_rule curr_rules;
>> int i, ret;
>> @@ -268,20 +269,22 @@ int atomisp_freq_scaling(struct atomisp_device *isp,
>> ATOMISP_USE_YUVPP(asd))
>> isp->dfs = &dfs_config_cht_soc;
>>
>> - if (isp->dfs->lowest_freq == 0 || isp->dfs->max_freq_at_vmin == 0 ||
>> - isp->dfs->highest_freq == 0 || isp->dfs->dfs_table_size == 0 ||
>> - !isp->dfs->dfs_table) {
>> + dfs = isp->dfs;
>> +
>> + if (dfs->lowest_freq == 0 || dfs->max_freq_at_vmin == 0 ||
>> + dfs->highest_freq == 0 || dfs->dfs_table_size == 0 ||
>> + !dfs->dfs_table) {
>> dev_err(isp->dev, "DFS configuration is invalid.\n");
>> return -EINVAL;
>> }
>>
>> if (mode == ATOMISP_DFS_MODE_LOW) {
>> - new_freq = isp->dfs->lowest_freq;
>> + new_freq = dfs->lowest_freq;
>> goto done;
>> }
>>
>> if (mode == ATOMISP_DFS_MODE_MAX) {
>> - new_freq = isp->dfs->highest_freq;
>> + new_freq = dfs->highest_freq;
>> goto done;
>> }
>>
>> @@ -307,26 +310,26 @@ int atomisp_freq_scaling(struct atomisp_device *isp,
>> }
>>
>> /* search for the target frequency by looping freq rules*/
>> - for (i = 0; i < isp->dfs->dfs_table_size; i++) {
>> - if (curr_rules.width != isp->dfs->dfs_table[i].width &&
>> - isp->dfs->dfs_table[i].width != ISP_FREQ_RULE_ANY)
>> + for (i = 0; i < dfs->dfs_table_size; i++) {
>> + if (curr_rules.width != dfs->dfs_table[i].width &&
>> + dfs->dfs_table[i].width != ISP_FREQ_RULE_ANY)
>> continue;
>> - if (curr_rules.height != isp->dfs->dfs_table[i].height &&
>> - isp->dfs->dfs_table[i].height != ISP_FREQ_RULE_ANY)
>> + if (curr_rules.height != dfs->dfs_table[i].height &&
>> + dfs->dfs_table[i].height != ISP_FREQ_RULE_ANY)
>> continue;
>> - if (curr_rules.fps != isp->dfs->dfs_table[i].fps &&
>> - isp->dfs->dfs_table[i].fps != ISP_FREQ_RULE_ANY)
>> + if (curr_rules.fps != dfs->dfs_table[i].fps &&
>> + dfs->dfs_table[i].fps != ISP_FREQ_RULE_ANY)
>> continue;
>> - if (curr_rules.run_mode != isp->dfs->dfs_table[i].run_mode &&
>> - isp->dfs->dfs_table[i].run_mode != ISP_FREQ_RULE_ANY)
>> + if (curr_rules.run_mode != dfs->dfs_table[i].run_mode &&
>> + dfs->dfs_table[i].run_mode != ISP_FREQ_RULE_ANY)
>> continue;
>> break;
>> }
>
>>
>> - if (i == isp->dfs->dfs_table_size)
>> - new_freq = isp->dfs->max_freq_at_vmin;
>> + if (i == dfs->dfs_table_size)
>> + new_freq = dfs->max_freq_at_vmin;
>> else
>> - new_freq = isp->dfs->dfs_table[i].isp_freq;
>> + new_freq = dfs->dfs_table[i].isp_freq;
>>
>
> you can eliminate the last block by setting
>
> new_freq = dfs->max_freq_at_vmin;
>
> for(i=0;....) {
> ....
> new_freq = dfs->dfs_table[i].isp_freq;
> break;
> }
Yes, it could be. I will make another patch to improve it as your comment.
>
> unfortunately i have no good idea how to make the loop more readable.
I am not sure whether the for-loop is possible to improve for
readability or not. :-)
Thanks for comment.
Regards,
Daeseok.
>
>
> re,
> wh
>
>
>> done:
>> dev_dbg(isp->dev, "DFS target frequency=%d.\n", new_freq);
