diff mbox

[2/2] drm/vc4: Add support for interlaced modes on HDMI.

Message ID 20160929022045.24813-2-eric@anholt.net (mailing list archive)
State New, archived
Headers show

Commit Message

Eric Anholt Sept. 29, 2016, 2:20 a.m. UTC 
We just needed to initialize a few more fields.

Signed-off-by: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/vc4/vc4_crtc.c | 17 ++++++++++++++---
 drivers/gpu/drm/vc4/vc4_hdmi.c | 12 ++++++++----
 drivers/gpu/drm/vc4/vc4_regs.h |  3 +++
 3 files changed, 25 insertions(+), 7 deletions(-)

Comments

yao mark Sept. 29, 2016, 2:36 a.m. UTC | #1 
On 2016年09月29日 10:20, Eric Anholt wrote:
> We just needed to initialize a few more fields.
>
> Signed-off-by: Eric Anholt <eric@anholt.net>
> ---
>   drivers/gpu/drm/vc4/vc4_crtc.c | 17 ++++++++++++++---
>   drivers/gpu/drm/vc4/vc4_hdmi.c | 12 ++++++++----
>   drivers/gpu/drm/vc4/vc4_regs.h |  3 +++
>   3 files changed, 25 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index 8fc2b731b59a..d575f8aa3273 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -428,13 +428,24 @@ static void vc4_crtc_mode_set_nofb(struct drm_crtc *crtc)
>   			   VC4_SET_FIELD(mode->vsync_start - mode->vdisplay,
>   					 PV_VERTB_VFP) |
>   			   VC4_SET_FIELD(vactive, PV_VERTB_VACTIVE));
> +
> +		/* We set up first field even mode for HDMI.  VEC's
> +		 * NTSC mode would want first field odd instead, once
> +		 * we support it (to do so, set ODD_FIRST and put the
> +		 * delay in VSYNCD_EVEN instead).
> +		 */
> +		CRTC_WRITE(PV_V_CONTROL,
> +			   PV_VCONTROL_CONTINUOUS |
> +			   PV_VCONTROL_INTERLACE |
> +			   VC4_SET_FIELD(mode->htotal / 2,
> +					 PV_VCONTROL_ODD_DELAY));
> +		CRTC_WRITE(PV_VSYNCD_EVEN, 0);
> +	} else {
> +		CRTC_WRITE(PV_V_CONTROL, PV_VCONTROL_CONTINUOUS);
>   	}
>   
>   	CRTC_WRITE(PV_HACT_ACT, mode->hdisplay);
>   
> -	CRTC_WRITE(PV_V_CONTROL,
> -		   PV_VCONTROL_CONTINUOUS |
> -		   (interlace ? PV_VCONTROL_INTERLACE : 0));
>   
>   	CRTC_WRITE(PV_CONTROL,
>   		   VC4_SET_FIELD(format, PV_CONTROL_FORMAT) |
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index 5770d6704f4b..6095e48fcf46 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -246,7 +246,7 @@ static struct drm_connector *vc4_hdmi_connector_init(struct drm_device *dev,
>   	connector->polled = (DRM_CONNECTOR_POLL_CONNECT |
>   			     DRM_CONNECTOR_POLL_DISCONNECT);
>   
> -	connector->interlace_allowed = 0;
> +	connector->interlace_allowed = true;
>   	connector->doublescan_allowed = 0;
>   
>   	drm_mode_connector_attach_encoder(connector, encoder);
> @@ -278,8 +278,8 @@ static void vc4_hdmi_encoder_mode_set(struct drm_encoder *encoder,
>   	bool debug_dump_regs = false;
>   	bool hsync_pos = mode->flags & DRM_MODE_FLAG_PHSYNC;
>   	bool vsync_pos = mode->flags & DRM_MODE_FLAG_PVSYNC;
> -	u32 vactive = (mode->vdisplay >>
> -		       ((mode->flags & DRM_MODE_FLAG_INTERLACE) ? 1 : 0));
> +	bool interlaced = mode->flags & DRM_MODE_FLAG_INTERLACE;
> +	u32 vactive = mode->vdisplay >> interlaced;

How about use mode->crtc_vdisplay:

see this:
drm_mode_set_crtcinfo()

     if (p->flags & DRM_MODE_FLAG_INTERLACE) {
         if (adjust_flags & CRTC_INTERLACE_HALVE_V) {
             p->crtc_vdisplay /= 2;
             p->crtc_vsync_start /= 2;
             p->crtc_vsync_end /= 2;
             p->crtc_vtotal /= 2;
         }
     }

Thanks

>   	u32 verta = (VC4_SET_FIELD(mode->vsync_end - mode->vsync_start,
>   				   VC4_HDMI_VERTA_VSP) |
>   		     VC4_SET_FIELD(mode->vsync_start - mode->vdisplay,
> @@ -288,6 +288,10 @@ static void vc4_hdmi_encoder_mode_set(struct drm_encoder *encoder,
>   	u32 vertb = (VC4_SET_FIELD(0, VC4_HDMI_VERTB_VSPO) |
>   		     VC4_SET_FIELD(mode->vtotal - mode->vsync_end,
>   				   VC4_HDMI_VERTB_VBP));
> +	u32 vertb_even = (VC4_SET_FIELD(0, VC4_HDMI_VERTB_VSPO) |
> +			  VC4_SET_FIELD(mode->vtotal - mode->vsync_end -
> +					interlaced,
> +					VC4_HDMI_VERTB_VBP));
>   
>   	if (debug_dump_regs) {
>   		DRM_INFO("HDMI regs before:\n");
> @@ -319,7 +323,7 @@ static void vc4_hdmi_encoder_mode_set(struct drm_encoder *encoder,
>   	HDMI_WRITE(VC4_HDMI_VERTA0, verta);
>   	HDMI_WRITE(VC4_HDMI_VERTA1, verta);
>   
> -	HDMI_WRITE(VC4_HDMI_VERTB0, vertb);
> +	HDMI_WRITE(VC4_HDMI_VERTB0, vertb_even);
>   	HDMI_WRITE(VC4_HDMI_VERTB1, vertb);
>   
>   	HD_WRITE(VC4_HD_VID_CTL,
> diff --git a/drivers/gpu/drm/vc4/vc4_regs.h b/drivers/gpu/drm/vc4/vc4_regs.h
> index 160942a9180e..fec7b5ef058b 100644
> --- a/drivers/gpu/drm/vc4/vc4_regs.h
> +++ b/drivers/gpu/drm/vc4/vc4_regs.h
> @@ -183,6 +183,9 @@
>   # define PV_CONTROL_EN				BIT(0)
>   
>   #define PV_V_CONTROL				0x04
> +# define PV_VCONTROL_ODD_DELAY_MASK		VC4_MASK(22, 6)
> +# define PV_VCONTROL_ODD_DELAY_SHIFT		6
> +# define PV_VCONTROL_ODD_FIRST			BIT(5)
>   # define PV_VCONTROL_INTERLACE			BIT(4)
>   # define PV_VCONTROL_CONTINUOUS			BIT(1)
>   # define PV_VCONTROL_VIDEN			BIT(0)
Eric Anholt Sept. 29, 2016, 2:53 a.m. UTC | #2 
Mark yao <mark.yao@rock-chips.com> writes:

> On 2016年09月29日 10:20, Eric Anholt wrote:
>> We just needed to initialize a few more fields.
>>
>> Signed-off-by: Eric Anholt <eric@anholt.net>
>> ---
>>   drivers/gpu/drm/vc4/vc4_crtc.c | 17 ++++++++++++++---
>>   drivers/gpu/drm/vc4/vc4_hdmi.c | 12 ++++++++----
>>   drivers/gpu/drm/vc4/vc4_regs.h |  3 +++
>>   3 files changed, 25 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
>> index 8fc2b731b59a..d575f8aa3273 100644
>> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
>> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
>> @@ -428,13 +428,24 @@ static void vc4_crtc_mode_set_nofb(struct drm_crtc *crtc)
>>   			   VC4_SET_FIELD(mode->vsync_start - mode->vdisplay,
>>   					 PV_VERTB_VFP) |
>>   			   VC4_SET_FIELD(vactive, PV_VERTB_VACTIVE));
>> +
>> +		/* We set up first field even mode for HDMI.  VEC's
>> +		 * NTSC mode would want first field odd instead, once
>> +		 * we support it (to do so, set ODD_FIRST and put the
>> +		 * delay in VSYNCD_EVEN instead).
>> +		 */
>> +		CRTC_WRITE(PV_V_CONTROL,
>> +			   PV_VCONTROL_CONTINUOUS |
>> +			   PV_VCONTROL_INTERLACE |
>> +			   VC4_SET_FIELD(mode->htotal / 2,
>> +					 PV_VCONTROL_ODD_DELAY));
>> +		CRTC_WRITE(PV_VSYNCD_EVEN, 0);
>> +	} else {
>> +		CRTC_WRITE(PV_V_CONTROL, PV_VCONTROL_CONTINUOUS);
>>   	}
>>   
>>   	CRTC_WRITE(PV_HACT_ACT, mode->hdisplay);
>>   
>> -	CRTC_WRITE(PV_V_CONTROL,
>> -		   PV_VCONTROL_CONTINUOUS |
>> -		   (interlace ? PV_VCONTROL_INTERLACE : 0));
>>   
>>   	CRTC_WRITE(PV_CONTROL,
>>   		   VC4_SET_FIELD(format, PV_CONTROL_FORMAT) |
>> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
>> index 5770d6704f4b..6095e48fcf46 100644
>> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
>> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
>> @@ -246,7 +246,7 @@ static struct drm_connector *vc4_hdmi_connector_init(struct drm_device *dev,
>>   	connector->polled = (DRM_CONNECTOR_POLL_CONNECT |
>>   			     DRM_CONNECTOR_POLL_DISCONNECT);
>>   
>> -	connector->interlace_allowed = 0;
>> +	connector->interlace_allowed = true;
>>   	connector->doublescan_allowed = 0;
>>   
>>   	drm_mode_connector_attach_encoder(connector, encoder);
>> @@ -278,8 +278,8 @@ static void vc4_hdmi_encoder_mode_set(struct drm_encoder *encoder,
>>   	bool debug_dump_regs = false;
>>   	bool hsync_pos = mode->flags & DRM_MODE_FLAG_PHSYNC;
>>   	bool vsync_pos = mode->flags & DRM_MODE_FLAG_PVSYNC;
>> -	u32 vactive = (mode->vdisplay >>
>> -		       ((mode->flags & DRM_MODE_FLAG_INTERLACE) ? 1 : 0));
>> +	bool interlaced = mode->flags & DRM_MODE_FLAG_INTERLACE;
>> +	u32 vactive = mode->vdisplay >> interlaced;
>
> How about use mode->crtc_vdisplay:
>
> see this:
> drm_mode_set_crtcinfo()
>
>      if (p->flags & DRM_MODE_FLAG_INTERLACE) {
>          if (adjust_flags & CRTC_INTERLACE_HALVE_V) {
>              p->crtc_vdisplay /= 2;
>              p->crtc_vsync_start /= 2;
>              p->crtc_vsync_end /= 2;
>              p->crtc_vtotal /= 2;
>          }
>      }

That would require setting up the adjust_flags, and I thought in the DRM
we were trying to move away from using it.  Also it would be pretty
strange to use just this one field from crtc_*.
diff mbox

Patch

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 8fc2b731b59a..d575f8aa3273 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -428,13 +428,24 @@  static void vc4_crtc_mode_set_nofb(struct drm_crtc *crtc)
 			   VC4_SET_FIELD(mode->vsync_start - mode->vdisplay,
 					 PV_VERTB_VFP) |
 			   VC4_SET_FIELD(vactive, PV_VERTB_VACTIVE));
+
+		/* We set up first field even mode for HDMI.  VEC's
+		 * NTSC mode would want first field odd instead, once
+		 * we support it (to do so, set ODD_FIRST and put the
+		 * delay in VSYNCD_EVEN instead).
+		 */
+		CRTC_WRITE(PV_V_CONTROL,
+			   PV_VCONTROL_CONTINUOUS |
+			   PV_VCONTROL_INTERLACE |
+			   VC4_SET_FIELD(mode->htotal / 2,
+					 PV_VCONTROL_ODD_DELAY));
+		CRTC_WRITE(PV_VSYNCD_EVEN, 0);
+	} else {
+		CRTC_WRITE(PV_V_CONTROL, PV_VCONTROL_CONTINUOUS);
 	}
 
 	CRTC_WRITE(PV_HACT_ACT, mode->hdisplay);
 
-	CRTC_WRITE(PV_V_CONTROL,
-		   PV_VCONTROL_CONTINUOUS |
-		   (interlace ? PV_VCONTROL_INTERLACE : 0));
 
 	CRTC_WRITE(PV_CONTROL,
 		   VC4_SET_FIELD(format, PV_CONTROL_FORMAT) |
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 5770d6704f4b..6095e48fcf46 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -246,7 +246,7 @@  static struct drm_connector *vc4_hdmi_connector_init(struct drm_device *dev,
 	connector->polled = (DRM_CONNECTOR_POLL_CONNECT |
 			     DRM_CONNECTOR_POLL_DISCONNECT);
 
-	connector->interlace_allowed = 0;
+	connector->interlace_allowed = true;
 	connector->doublescan_allowed = 0;
 
 	drm_mode_connector_attach_encoder(connector, encoder);
@@ -278,8 +278,8 @@  static void vc4_hdmi_encoder_mode_set(struct drm_encoder *encoder,
 	bool debug_dump_regs = false;
 	bool hsync_pos = mode->flags & DRM_MODE_FLAG_PHSYNC;
 	bool vsync_pos = mode->flags & DRM_MODE_FLAG_PVSYNC;
-	u32 vactive = (mode->vdisplay >>
-		       ((mode->flags & DRM_MODE_FLAG_INTERLACE) ? 1 : 0));
+	bool interlaced = mode->flags & DRM_MODE_FLAG_INTERLACE;
+	u32 vactive = mode->vdisplay >> interlaced;
 	u32 verta = (VC4_SET_FIELD(mode->vsync_end - mode->vsync_start,
 				   VC4_HDMI_VERTA_VSP) |
 		     VC4_SET_FIELD(mode->vsync_start - mode->vdisplay,
@@ -288,6 +288,10 @@  static void vc4_hdmi_encoder_mode_set(struct drm_encoder *encoder,
 	u32 vertb = (VC4_SET_FIELD(0, VC4_HDMI_VERTB_VSPO) |
 		     VC4_SET_FIELD(mode->vtotal - mode->vsync_end,
 				   VC4_HDMI_VERTB_VBP));
+	u32 vertb_even = (VC4_SET_FIELD(0, VC4_HDMI_VERTB_VSPO) |
+			  VC4_SET_FIELD(mode->vtotal - mode->vsync_end -
+					interlaced,
+					VC4_HDMI_VERTB_VBP));
 
 	if (debug_dump_regs) {
 		DRM_INFO("HDMI regs before:\n");
@@ -319,7 +323,7 @@  static void vc4_hdmi_encoder_mode_set(struct drm_encoder *encoder,
 	HDMI_WRITE(VC4_HDMI_VERTA0, verta);
 	HDMI_WRITE(VC4_HDMI_VERTA1, verta);
 
-	HDMI_WRITE(VC4_HDMI_VERTB0, vertb);
+	HDMI_WRITE(VC4_HDMI_VERTB0, vertb_even);
 	HDMI_WRITE(VC4_HDMI_VERTB1, vertb);
 
 	HD_WRITE(VC4_HD_VID_CTL,
diff --git a/drivers/gpu/drm/vc4/vc4_regs.h b/drivers/gpu/drm/vc4/vc4_regs.h
index 160942a9180e..fec7b5ef058b 100644
--- a/drivers/gpu/drm/vc4/vc4_regs.h
+++ b/drivers/gpu/drm/vc4/vc4_regs.h
@@ -183,6 +183,9 @@ 
 # define PV_CONTROL_EN				BIT(0)
 
 #define PV_V_CONTROL				0x04
+# define PV_VCONTROL_ODD_DELAY_MASK		VC4_MASK(22, 6)
+# define PV_VCONTROL_ODD_DELAY_SHIFT		6
+# define PV_VCONTROL_ODD_FIRST			BIT(5)
 # define PV_VCONTROL_INTERLACE			BIT(4)
 # define PV_VCONTROL_CONTINUOUS			BIT(1)
 # define PV_VCONTROL_VIDEN			BIT(0)