[v3,09/10] drm/rockchip: analogix_dp: update the comments about why need to hardcode VOP output mode
diff mbox

Message ID 1465904797-1117-1-git-send-email-ykk@rock-chips.com
State New
Headers show

Commit Message

Yakir Yang June 14, 2016, 11:46 a.m. UTC
The hardware IC designed that VOP must output the RGB10 video format to
eDP contoller, and if eDP panel only support RGB8, then eDP contoller
should cut down the video data, not via VOP contoller, that's why we need
to hardcode the VOP output mode to RGA10 here.

Signed-off-by: Yakir Yang <ykk@rock-chips.com>
Acked-by: Mark Yao <mark.yao@rock-chips.com>
Reviewed-by: Tomasz Figa <tomasz.figa@chromium.com>
---
Changes in v3:
- Add the reviewed flag from Tomasz.
    [https://chromium-review.googlesource.com/#/c/346853/12]
- Add the acked flag from Mark.

Changes in v2:
- new patch in v2

 drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

Comments

Sean Paul June 23, 2016, 2:22 p.m. UTC | #1
On Tue, Jun 14, 2016 at 7:46 AM, Yakir Yang <ykk@rock-chips.com> wrote:
> The hardware IC designed that VOP must output the RGB10 video format to
> eDP contoller, and if eDP panel only support RGB8, then eDP contoller
> should cut down the video data, not via VOP contoller, that's why we need
> to hardcode the VOP output mode to RGA10 here.
>
> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> Acked-by: Mark Yao <mark.yao@rock-chips.com>
> Reviewed-by: Tomasz Figa <tomasz.figa@chromium.com>
> ---
> Changes in v3:
> - Add the reviewed flag from Tomasz.
>     [https://chromium-review.googlesource.com/#/c/346853/12]
> - Add the acked flag from Mark.
>
> Changes in v2:
> - new patch in v2
>
>  drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> index 95a6f60..2ceb3f9 100644
> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> @@ -173,17 +173,11 @@ rockchip_dp_drm_encoder_atomic_check(struct drm_encoder *encoder,
>         int ret;
>
>         /*
> -        * FIXME(Yakir): driver should configure the CRTC output video
> -        * mode with the display information which indicated the monitor
> -        * support colorimetry.
> -        *
> -        * But don't know why the CRTC driver seems could only output the
> -        * RGBaaa rightly. For example, if connect the "innolux,n116bge"
> -        * eDP screen, EDID would indicated that screen only accepted the
> -        * 6bpc mode. But if I configure CRTC to RGB666 output, then eDP
> -        * screen would show a blue picture (RGB888 show a green picture).
> -        * But if I configure CTRC to RGBaaa, and eDP driver still keep
> -        * RGB666 input video mode, then screen would works prefect.
> +        * The hardware IC designed that VOP must output the RGB10 video
> +        * format to eDP contoller, and if eDP panel only support RGB8,
> +        * then eDP contoller should cut down the video data, not via VOP
> +        * contoller, that's why we need to hardcode the VOP output mode
> +        * to RGA10 here.

s/contoller/controller/

>          */
>
>         ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, encoder);
> --
> 1.9.1
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Yakir Yang June 29, 2016, 6:42 a.m. UTC | #2
Sean,

On 06/23/2016 10:22 PM, Sean Paul wrote:
> On Tue, Jun 14, 2016 at 7:46 AM, Yakir Yang <ykk@rock-chips.com> wrote:
>> The hardware IC designed that VOP must output the RGB10 video format to
>> eDP contoller, and if eDP panel only support RGB8, then eDP contoller
>> should cut down the video data, not via VOP contoller, that's why we need
>> to hardcode the VOP output mode to RGA10 here.
>>
>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
>> Acked-by: Mark Yao <mark.yao@rock-chips.com>
>> Reviewed-by: Tomasz Figa <tomasz.figa@chromium.com>
>> ---
>> Changes in v3:
>> - Add the reviewed flag from Tomasz.
>>      [https://chromium-review.googlesource.com/#/c/346853/12]
>> - Add the acked flag from Mark.
>>
>> Changes in v2:
>> - new patch in v2
>>
>>   drivers/gpu/drm/rockchip/analogix_dp-rockchip.c | 16 +++++-----------
>>   1 file changed, 5 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>> index 95a6f60..2ceb3f9 100644
>> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
>> @@ -173,17 +173,11 @@ rockchip_dp_drm_encoder_atomic_check(struct drm_encoder *encoder,
>>          int ret;
>>
>>          /*
>> -        * FIXME(Yakir): driver should configure the CRTC output video
>> -        * mode with the display information which indicated the monitor
>> -        * support colorimetry.
>> -        *
>> -        * But don't know why the CRTC driver seems could only output the
>> -        * RGBaaa rightly. For example, if connect the "innolux,n116bge"
>> -        * eDP screen, EDID would indicated that screen only accepted the
>> -        * 6bpc mode. But if I configure CRTC to RGB666 output, then eDP
>> -        * screen would show a blue picture (RGB888 show a green picture).
>> -        * But if I configure CTRC to RGBaaa, and eDP driver still keep
>> -        * RGB666 input video mode, then screen would works prefect.
>> +        * The hardware IC designed that VOP must output the RGB10 video
>> +        * format to eDP contoller, and if eDP panel only support RGB8,
>> +        * then eDP contoller should cut down the video data, not via VOP
>> +        * contoller, that's why we need to hardcode the VOP output mode
>> +        * to RGA10 here.
> s/contoller/controller/
>

Done,

>>           */
>>
>>          ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, encoder);
>> --
>> 1.9.1
>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>

Patch
diff mbox

diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
index 95a6f60..2ceb3f9 100644
--- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
+++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
@@ -173,17 +173,11 @@  rockchip_dp_drm_encoder_atomic_check(struct drm_encoder *encoder,
 	int ret;
 
 	/*
-	 * FIXME(Yakir): driver should configure the CRTC output video
-	 * mode with the display information which indicated the monitor
-	 * support colorimetry.
-	 *
-	 * But don't know why the CRTC driver seems could only output the
-	 * RGBaaa rightly. For example, if connect the "innolux,n116bge"
-	 * eDP screen, EDID would indicated that screen only accepted the
-	 * 6bpc mode. But if I configure CRTC to RGB666 output, then eDP
-	 * screen would show a blue picture (RGB888 show a green picture).
-	 * But if I configure CTRC to RGBaaa, and eDP driver still keep
-	 * RGB666 input video mode, then screen would works prefect.
+	 * The hardware IC designed that VOP must output the RGB10 video
+	 * format to eDP contoller, and if eDP panel only support RGB8,
+	 * then eDP contoller should cut down the video data, not via VOP
+	 * contoller, that's why we need to hardcode the VOP output mode
+	 * to RGA10 here.
 	 */
 
 	ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, encoder);