Re: [PATCH 3/4] drm/panel: sitronix-st7789v: add support for partial mode

[Michael Riesch <michael.riesch@wolfvision.net>]

Hi Neil,

On 8/4/23 10:41, Neil Armstrong wrote:
> Hi Michael,
> 
> On 18/07/2023 17:31, Michael Riesch wrote:
>> The ST7789V controller features support for the partial mode. Here,
>> the area to be displayed can be restricted in one direction (by default,
>> in vertical direction). This is useful for panels that are partial >
>> occluded by design. Add support for the partial mode.
> 
> Could you send a v2 with a comment in the code as Maxime suggests ?

Sure thing! I must admit that I do not understand his concerns exactly,
though.

@Maxime: I can prepare a suggestion but feel free to tell me the exact
wording at the preferred position.

Best regards,
Michael

> 
> Thanks,
> Neil
> 
>>
>> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
>> ---
>>   drivers/gpu/drm/panel/panel-sitronix-st7789v.c | 38
>> ++++++++++++++++++++++++--
>>   1 file changed, 36 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
>> b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
>> index d16d17f21d92..729d8d7dbf7f 100644
>> --- a/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
>> +++ b/drivers/gpu/drm/panel/panel-sitronix-st7789v.c
>> @@ -118,6 +118,9 @@ struct st7789_panel_info {
>>       u32 bus_format;
>>       u32 bus_flags;
>>       bool invert_mode;
>> +    bool partial_mode;
>> +    u16 partial_start;
>> +    u16 partial_end;
>>   };
>>     struct st7789v {
>> @@ -330,9 +333,14 @@ static int st7789v_get_modes(struct drm_panel
>> *panel,
>>   static int st7789v_prepare(struct drm_panel *panel)
>>   {
>>       struct st7789v *ctx = panel_to_st7789v(panel);
>> -    u8 pixel_fmt, polarity;
>> +    u8 mode, pixel_fmt, polarity;
>>       int ret;
>>   +    if (!ctx->info->partial_mode)
>> +        mode = ST7789V_RGBCTRL_WO;
>> +    else
>> +        mode = 0;
>> +
>>       switch (ctx->info->bus_format) {
>>       case MEDIA_BUS_FMT_RGB666_1X18:
>>           pixel_fmt = MIPI_DCS_PIXEL_FMT_18BIT;
>> @@ -472,6 +480,32 @@ static int st7789v_prepare(struct drm_panel *panel)
>>                           MIPI_DCS_EXIT_INVERT_MODE));
>>       }
>>   +    if (ctx->info->partial_mode) {
>> +        u8 area_data[4] = {
>> +            (ctx->info->partial_start >> 8) & 0xff,
>> +            (ctx->info->partial_start >> 0) & 0xff,
>> +            ((ctx->info->partial_end - 1) >> 8) & 0xff,
>> +            ((ctx->info->partial_end - 1) >> 0) & 0xff,
>> +        };
>> +
>> +        ST7789V_TEST(ret, st7789v_write_command(
>> +                      ctx, MIPI_DCS_ENTER_PARTIAL_MODE));
>> +
>> +        ST7789V_TEST(ret, st7789v_write_command(
>> +                      ctx, MIPI_DCS_SET_PAGE_ADDRESS));
>> +        ST7789V_TEST(ret, st7789v_write_data(ctx, area_data[0]));
>> +        ST7789V_TEST(ret, st7789v_write_data(ctx, area_data[1]));
>> +        ST7789V_TEST(ret, st7789v_write_data(ctx, area_data[2]));
>> +        ST7789V_TEST(ret, st7789v_write_data(ctx, area_data[3]));
>> +
>> +        ST7789V_TEST(ret, st7789v_write_command(
>> +                      ctx, MIPI_DCS_SET_PARTIAL_ROWS));
>> +        ST7789V_TEST(ret, st7789v_write_data(ctx, area_data[0]));
>> +        ST7789V_TEST(ret, st7789v_write_data(ctx, area_data[1]));
>> +        ST7789V_TEST(ret, st7789v_write_data(ctx, area_data[2]));
>> +        ST7789V_TEST(ret, st7789v_write_data(ctx, area_data[3]));
>> +    }
>> +
>>       ST7789V_TEST(ret, st7789v_write_command(ctx, ST7789V_RAMCTRL_CMD));
>>       ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_RAMCTRL_DM_RGB |
>>                            ST7789V_RAMCTRL_RM_RGB));
>> @@ -479,7 +513,7 @@ static int st7789v_prepare(struct drm_panel *panel)
>>                            ST7789V_RAMCTRL_MAGIC));
>>         ST7789V_TEST(ret, st7789v_write_command(ctx,
>> ST7789V_RGBCTRL_CMD));
>> -    ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_RGBCTRL_WO |
>> +    ST7789V_TEST(ret, st7789v_write_data(ctx, mode |
>>                            ST7789V_RGBCTRL_RCM(2) |
>>                            polarity));
>>       ST7789V_TEST(ret, st7789v_write_data(ctx, ST7789V_RGBCTRL_VBP(8)));
>>
>